From 023fc779655c42aa0762ca0895b304c6374fcc95 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 12:46:07 -0400 Subject: [PATCH 01/17] add a plan for adding memory to grid objects --- .../260725-grid-resource-lifecycle-plan.md | 342 ++++++++++++++++++ 1 file changed, 342 insertions(+) create mode 100644 plan_histories/260725-grid-resource-lifecycle-plan.md diff --git a/plan_histories/260725-grid-resource-lifecycle-plan.md b/plan_histories/260725-grid-resource-lifecycle-plan.md new file mode 100644 index 000000000..8a803dfa7 --- /dev/null +++ b/plan_histories/260725-grid-resource-lifecycle-plan.md @@ -0,0 +1,342 @@ +# Grid Resource Lifecycle and Serialization Plan + +## Goal + +Extend the grid API so that `DataBox` can manage grids that may eventually own +dynamic host or device data. Add serialization, pointer relocation, device-copy, +and finalization operations to the current grid types, and thread those +operations through `PiecewiseGrid1D` and `DataBox`. + +The current grids contain only inline data, so most initial implementations are +trivial. The design must nevertheless exercise the complete ownership path so a +future dynamically allocated grid does not require another `DataBox` API +change. + +## Resource-lifecycle policy + +Apply grid serialization, device copying, pointer relocation, and finalization +to every grid slot in `[0, rank)`, regardless of its `IndexType`. + +`IndexType` controls interpolation semantics, not resource lifecycle. Processing +all grid slots: + +- keeps the lifecycle loops simple and consistent; +- prevents an inactive grid from retaining an unrelocated host pointer in a + device-side `DataBox`; +- prevents leaks when a dimension changes from interpolated to indexed or + named; +- avoids making resource ownership depend on mutable interpolation metadata. + +This requires every default-constructed grid to be a valid empty object. +`PiecewiseGrid1D` must therefore initialize `NGRIDS_` to zero and initialize any +other state required by its empty operations. + +The following invariants should hold for every supported grid: + +- default construction creates a valid empty grid; +- empty-grid serialization and device copying are valid; +- `setPointer()` may consume zero bytes; +- `finalize()` is idempotent and safe on empty or unmanaged storage; +- `getOnDevice()` performs a deep copy of any owned allocation; +- deserialized storage is unmanaged and refers into the supplied byte buffer. + +## Public grid API + +Add the following interface to `RegularGrid1D` and `PiecewiseGrid1D`: + +```cpp +std::size_t serializedSizeInBytes() const; +std::size_t serialize(char *dst) const; +std::size_t deSerialize(char *src); +std::size_t setPointer(char *src); +GridType getOnDevice() const; +void finalize(); +``` + +Preserve the existing `deSerialize` capitalization used by `DataBox` unless a +separate project-wide API rename is undertaken. + +### Serialization contract + +Each serializable object uses this recursive representation: + +```text +[object bytes][owned payload or recursively serialized child objects] +``` + +- `serializedSizeInBytes()` includes `sizeof(*this)` and all subordinate + serialized storage. +- `serialize()` writes the complete representation and returns the number of + bytes written. +- `deSerialize()` copies the object bytes, calls `setPointer()` on the remainder, + and returns the total number of bytes consumed. +- `setPointer()` assumes the object's inline metadata has already been restored. + It relocates owned pointers into the supplied buffer and processes subordinate + records, returning the number of payload bytes consumed. + +Offsets returned by every method are accumulated by callers. Tests must verify +that calculated, written, and consumed sizes agree. + +## `RegularGrid1D` implementation + +Implement the new operations in `spiner/regular_grid_1d.hpp`. + +Because all current state is inline: + +- `serializedSizeInBytes()` returns `sizeof(*this)`; +- `serialize()` copies `sizeof(*this)` bytes to `dst`; +- `deSerialize()` copies `sizeof(*this)` bytes from `src`, then calls + `setPointer(src + sizeof(*this))`; +- `setPointer()` returns zero; +- `getOnDevice()` returns a value copy of `*this`; +- `finalize()` is a no-op. + +Keep host-only memory-management routines undecorated unless the build requires +a portability annotation. Interpolation and access methods remain portable. + +## `PiecewiseGrid1D` implementation + +Implement the new operations in `spiner/piecewise_grid_1d.hpp`. + +1. Make the default constructor establish an empty state, including + `NGRIDS_ == 0`. +2. Serialize the `PiecewiseGrid1D` object followed by a complete serialized + record for each child in `[0, NGRIDS_)`. +3. In `setPointer()`, deserialize each child record in sequence. Calling the + child's `deSerialize()` is appropriate because each child record starts with + that child's object bytes. +4. In `getOnDevice()`, copy inline metadata and replace every active child with + `grids_[i].getOnDevice()`. +5. In `finalize()`, call `finalize()` on every child in `[0, NGRIDS_)`, then + leave the piecewise grid in a valid empty/finalized state. + +This recursive implementation is intentionally nontrivial even though +`RegularGrid1D` currently owns no allocation. It ensures that a future +pointer-owning child grid is handled automatically. + +Review empty-grid query behavior separately from lifecycle behavior. Methods +such as `min()`, `max()`, and `nPoints()` may continue to require a nonempty +grid, while serialization, device copying, and finalization must support an +empty grid. + +## `DataBox` implementation + +Update the existing lifecycle methods in `spiner/databox.hpp`. + +### Serialized representation + +Use the following layout: + +```text +[DataBox bytes][DataBox value data][serialized grid 0]...[serialized grid rank-1] +``` + +The grid records duplicate the inline grid metadata present in the raw +`DataBox` image. This small cost provides a uniform, independently testable grid +serialization contract and permits each grid to restore its own future +allocation without `DataBox` knowing its representation. + +### Size and serialization + +- Extend `serializedSizeInBytes()` by summing + `grids_[i].serializedSizeInBytes()` for every `i` in `[0, rank)`. +- Keep the prohibition against serializing device-resident `DataBox` values. +- After writing the existing `DataBox` header and value buffer, call + `grids_[i].serialize()` for every dimension and accumulate each returned + offset. + +### Deserialization and pointer relocation + +- Preserve the existing precondition that an owning, active `DataBox` cannot be + overwritten by deserialization. +- Copy the `DataBox` object bytes first so its dimensions, rank, and grid + metadata are available. +- Have `DataBox::setPointer()` relocate the value pointer and rebuild the + `PortableMDArray`, then call `grids_[i].deSerialize()` for every dimension. +- Mark the resulting value storage unmanaged, as today. +- Ensure each deserialized grid likewise regards memory inside the serialization + buffer as unmanaged. + +The byte buffer must outlive every `DataBox` deserialized from it. + +### Device copy + +Retain the current deep copy of the value buffer. After copying shape and index +metadata, replace every grid in `[0, rank)` with the result of that grid's +`getOnDevice()`. + +Avoid leaving a shallow host copy of any grid allocation in the returned +device-side `DataBox`, including for indexed or named dimensions. + +### Finalization + +Call `finalize()` on every grid in `[0, rank)` before freeing the `DataBox` +value allocation. Then clear the `DataBox` pointer/view state consistently with +its empty status. + +Grid finalization must be safe when `DataBox::finalize()` is reached during +allocation or resize of a default/empty grid. Preserve the current explicit +manual-lifetime model; introducing RAII ownership is outside this change. + +### Range replacement and index changes + +Since every slot participates in lifecycle management, stale grids will still +be finalized by the owning `DataBox`. Nevertheless, review these mutation +paths: + +- replacing a grid with `setRange()`; +- changing an interpolated dimension to indexed or named; +- `reset()`, shallow copy, slicing, assignment, and `copyShape()`. + +Document and test which object remains responsible for finalizing shallow grid +allocations. Do not silently introduce a second owner through ordinary copy or +assignment. A future owning grid will likely need an ownership-status field +parallel to `DataStatus`; its normal copies should be shallow/unmanaged, while +`getOnDevice()` is explicitly deep. + +## HDF5 behavior + +Do not change the HDF5 representation as part of this work. + +HDF5 represents meaningful interpolation metadata rather than the complete +in-memory object graph, so `DataBox::saveHDF()` and `loadHDF()` may continue to +visit only interpolated dimensions. They already delegate grid persistence to +the grid's `saveHDF()` and `loadHDF()` methods. A future grid that owns data must +extend those methods independently. + +Run the existing HDF5 round-trip tests as regressions. + +## Serialization compatibility documentation + +Cross-version serialization compatibility is explicitly not a requirement. No +version marker or compatibility reader is needed. + +Update `doc/sphinx/src/databox.rst` and add a concise comment next to the +serialization implementation stating: + +> DataBox serialization is intended for transient communication between +> processes using the same Spiner version and a compatible architecture. The +> binary representation is not guaranteed to be compatible across Spiner +> versions, architectures, compilers, or build configurations. It should not be +> used as a persistent storage format or a web interchange format. Use HDF5 for +> persistent, portable storage. + +Also document: + +- the revised `DataBox` and nested-grid layout; +- that grid records are emitted for every dimension in `[0, rank)`, including + indexed and named dimensions; +- that deserialized value and grid storage refer into an externally owned + buffer; +- that the buffer must remain alive and suitably aligned for the lifetime of + the deserialized object; +- that size calculation, serialization, and deserialization must use a + compatible Spiner build. + +The existing architecture and alignment limitations remain unchanged. + +## Testing plan + +### 1. `RegularGrid1D` unit tests + +- Round-trip a populated regular grid. +- Verify calculated, written, and consumed sizes are all `sizeof(grid)`. +- Verify equality, `x()`, `index()`, and `weights()` after deserialization. +- Exercise `setPointer()`, `getOnDevice()`, and repeated `finalize()` calls. +- Exercise the same lifecycle methods on a default-constructed grid. + +### 2. `PiecewiseGrid1D` unit tests + +- Verify a default piecewise grid is safely empty and supports all lifecycle + operations. +- Round-trip a multi-segment piecewise grid. +- Verify its serialized size includes its own header and every child record. +- Verify equality, point count, coordinate lookup, indexing, weights, and + interpolation behavior after deserialization. +- Verify device copying and repeated finalization recursively process children. + +### 3. Existing `DataBox` serialization tests + +Extend the serialization scenario in `test/test.cpp`: + +- update the expected size to include one serialized grid record per dimension; +- verify calculated, written, and consumed sizes agree; +- verify rank, dimensions, values, index types, and all grid ranges after + deserialization; +- retain the test that independently deserialized `DataBox` objects refer to + the same serialized buffer while remaining distinct C++ objects; +- add a mixture of interpolated, indexed, and named dimensions and verify all + grid records are processed; +- test empty/default grid slots explicitly. + +### 4. Device integration tests + +Extend the `DataBox::getOnDevice()` tests: + +- assign grids before copying; +- perform interpolation inside a portable kernel; +- verify both the values and grid data are usable on the device; +- include a non-interpolated dimension whose grid lifecycle method must still + be invoked; +- finalize the device and host objects independently. + +Run these tests with host-only portability and with an actual accelerator +backend where available. + +### 5. Test-only owning grid + +Add an `OwningTestGrid1D` in the test code. It should implement the grid +interface and own a small dynamically allocated coordinate or coefficient +array. Use it as `DataBox` to verify behavior that trivial +production grids cannot expose: + +- serialization includes the owned payload; +- deserialization relocates the grid pointer into the serialized buffer; +- `DataBox::setPointer()` reaches every grid slot; +- indexed and named dimensions do not retain stale original pointers; +- `getOnDevice()` deep-copies grid storage; +- interpolation on the device reads the device allocation; +- finalizing a `DataBox` releases both its grid allocations and value + allocation; +- empty/unmanaged finalization is harmless; +- shallow copies do not become independent owners; +- calculated, written, and consumed offsets remain identical at every nesting + level. + +This test double is the primary proof that all delegation paths have been +threaded correctly. + +### 6. Regression matrix + +Build and run: + +- CPU-only tests; +- Kokkos host tests; +- CUDA or another device backend where available; +- HDF5 disabled; +- HDF5 enabled, including existing regular and piecewise HDF5 round trips. + +Run formatting and the repository's normal compile/test checks after the +implementation. + +## Suggested implementation order + +1. Establish valid empty-grid invariants. +2. Add and test the trivial `RegularGrid1D` API. +3. Add and test recursive `PiecewiseGrid1D` behavior. +4. Thread all operations through every `DataBox` dimension. +5. Add the test-only owning grid and integration coverage. +6. Update serialization documentation and source comments. +7. Run the full build matrix and address ownership or portability failures. + +## Acceptance criteria + +- Both production grid types expose the complete lifecycle API. +- Every `DataBox` grid slot in `[0, rank)` participates in serialization, + deserialization, pointer relocation, device copying, and finalization. +- Static grids retain their current interpolation and HDF5 behavior. +- A test-only dynamically allocated grid works end-to-end through `DataBox`. +- Serialization byte counts agree at every layer. +- Device tests demonstrate that no grid retains a host-only pointer. +- The documentation clearly disclaims cross-version and cross-architecture + compatibility and recommends HDF5 for persistent storage. From e3278bf3b95b6152eeed4003d0a7082da23d5ef3 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 14:24:19 -0400 Subject: [PATCH 02/17] Grid files look good --- spiner/piecewise_grid_1d.hpp | 110 ++++++++++++++++++++++++++++++++++- spiner/regular_grid_1d.hpp | 36 +++++++++++- spiner/spiner_types.hpp | 15 ++++- 3 files changed, 156 insertions(+), 5 deletions(-) diff --git a/spiner/piecewise_grid_1d.hpp b/spiner/piecewise_grid_1d.hpp index 693ba84a8..3a89d44f3 100644 --- a/spiner/piecewise_grid_1d.hpp +++ b/spiner/piecewise_grid_1d.hpp @@ -15,7 +15,11 @@ // permit others to do so. //====================================================================== +// Generative AI was used to assist with modifications to this file. + #include +#include +#include #include #include #include @@ -46,6 +50,33 @@ class PiecewiseGrid1D { // This is functionally equivalent because grids_ will // be initialized to default values PORTABLE_INLINE_FUNCTION PiecewiseGrid1D() {} + PORTABLE_INLINE_FUNCTION + PiecewiseGrid1D(const PiecewiseGrid1D &) = default; + PORTABLE_INLINE_FUNCTION PiecewiseGrid1D & + operator=(const PiecewiseGrid1D &) = default; + PiecewiseGrid1D(PiecewiseGrid1D &&other) noexcept : NGRIDS_(other.NGRIDS_) { + for (int i = 0; i < NGRIDS_; ++i) { + grids_[i] = std::move(other.grids_[i]); + pointTotals_[i] = other.pointTotals_[i]; + } + if (other.dataStatus() != DataStatus::Unmanaged) { + other.NGRIDS_ = 0; + } + } + PiecewiseGrid1D &operator=(PiecewiseGrid1D &&other) noexcept { + if (this != &other) { + finalize(); + NGRIDS_ = other.NGRIDS_; + for (int i = 0; i < NGRIDS_; ++i) { + grids_[i] = std::move(other.grids_[i]); + pointTotals_[i] = other.pointTotals_[i]; + } + if (other.dataStatus() != DataStatus::Unmanaged) { + other.NGRIDS_ = 0; + } + } + return *this; + } PiecewiseGrid1D(const std::vector> grids) { NGRIDS_ = grids.size(); PORTABLE_ALWAYS_REQUIRE( @@ -107,6 +138,19 @@ class PiecewiseGrid1D { return ig; } + PORTABLE_INLINE_FUNCTION DataStatus dataStatus() const { + int status = DataStatus::Trivial; + for (int i = 0; i < NGRIDS_; ++i) { + int gs = static_cast(grids_[i].dataStatus()); + PORTABLE_REQUIRE( + !((status == static_cast(DataStatus::AllocatedHost)) && + (gs == static_cast(DataStatus::AllocatedDevice))), + "Can't mix allocatedhost/allocated device!"); + status = std::max(gs, status); + } + return static_cast(status); + } + PORTABLE_INLINE_FUNCTION T x(const int i) const { int ig = findGridFromGlobalIdx(i); return grids_[ig].x(i - pointTotals_[ig]); @@ -151,9 +195,69 @@ class PiecewiseGrid1D { } return false; } - PORTABLE_INLINE_FUNCTION bool isWellFormed() const { return !isnan(); } + PORTABLE_INLINE_FUNCTION bool isWellFormed() const { + return NGRIDS_ > 0 && !isnan(); + } PORTABLE_INLINE_FUNCTION int nGrids() const { return NGRIDS_; } + // Binary serialization is intended for transient communication between + // compatible Spiner builds, not as a persistent or portable file format. + std::size_t dynamicMemorySizeInBytes() const { + std::size_t size = 0; + for (int i = 0; i < NGRIDS_; ++i) { + size += grids_[i].dynamicMemorySizeInBytes(); + } + return size; + } + + std::size_t serializedSizeInBytes() const { + return sizeof(*this) + dynamicMemorySizeInBytes(); + } + + std::size_t dumpDynamicMemory(char *dst) const { + std::size_t offset = 0; + for (int i = 0; i < NGRIDS_; ++i) { + offset += grids_[i].dumpDynamicMemory(dst + offset); + } + return offset; + } + + std::size_t serialize(char *dst) const { + std::memcpy(dst, this, sizeof(*this)); + return sizeof(*this) + dumpDynamicMemory(dst + sizeof(*this)); + } + + std::size_t setPointer(char *src) { + std::size_t offset = 0; + for (int i = 0; i < NGRIDS_; ++i) { + offset += grids_[i].setPointer(src + offset); + } + return offset; + } + + std::size_t deSerialize(char *src) { + finalize(); // TODO(JMM): Maybe guard this finalize + std::memcpy(this, src, sizeof(*this)); + PORTABLE_REQUIRE(0 <= NGRIDS_ && NGRIDS_ <= NGRIDSMAX, + "Invalid number of piecewise grids"); + return sizeof(*this) + setPointer(src + sizeof(*this)); + } + + PiecewiseGrid1D getOnDevice() const { + PiecewiseGrid1D grid(*this); + for (int i = 0; i < NGRIDS_; ++i) { + grid.grids_[i] = grids_[i].getOnDevice(); + } + return grid; + } + + void finalize() { + for (int i = 0; i < NGRIDS_; ++i) { + grids_[i].finalize(); // TODO(JMM): Maybe guard this finalize + } + NGRIDS_ = 0; + } + #ifdef SPINER_USE_HDF inline herr_t saveHDF(hid_t loc, const std::string &name) const { static_assert( @@ -228,8 +332,8 @@ class PiecewiseGrid1D { } RegularGrid1D grids_[NGRIDSMAX]; - int pointTotals_[NGRIDSMAX]; - int NGRIDS_; + int pointTotals_[NGRIDSMAX]{}; + int NGRIDS_ = 0; }; } // namespace Spiner diff --git a/spiner/regular_grid_1d.hpp b/spiner/regular_grid_1d.hpp index d69076d65..b84b8abcf 100644 --- a/spiner/regular_grid_1d.hpp +++ b/spiner/regular_grid_1d.hpp @@ -1,7 +1,7 @@ #ifndef SPINER_REGULAR_GRID_1D_ #define SPINER_REGULAR_GRID_1D_ //====================================================================== -// © (or copyright) 2019-2023. Triad National Security, LLC. All rights +// © (or copyright) 2019-2026. Triad National Security, LLC. All rights // reserved. This program was produced under U.S. Government contract // 89233218CNA000001 for Los Alamos National Laboratory (LANL), which is // operated by Triad National Security, LLC for the U.S. Department of @@ -15,8 +15,12 @@ // permit others to do so. //====================================================================== +// Generative AI was used to assist with modifications to this file. + #include #include +#include +#include #include #include @@ -115,6 +119,36 @@ class RegularGrid1D { } PORTABLE_INLINE_FUNCTION bool isWellFormed() const { return !isnan(); } + PORTABLE_INLINE_FUNCTION DataStatus dataStatus() const { + return DataStatus::Trivial; + } + + // Binary serialization is intended for transient communication between + // compatible Spiner builds, not as a persistent or portable file format. + std::size_t dynamicMemorySizeInBytes() const { return 0; } + + std::size_t serializedSizeInBytes() const { + return sizeof(*this) + dynamicMemorySizeInBytes(); + } + + std::size_t dumpDynamicMemory(char *) const { return 0; } + + std::size_t serialize(char *dst) const { + std::memcpy(dst, this, sizeof(*this)); + return sizeof(*this) + dumpDynamicMemory(dst + sizeof(*this)); + } + + std::size_t setPointer(char *) { return 0; } + + std::size_t deSerialize(char *src) { + std::memcpy(this, src, sizeof(*this)); + return sizeof(*this); + } + + RegularGrid1D getOnDevice() const { return *this; } + + void finalize() {} + #ifdef SPINER_USE_HDF inline herr_t saveHDF(hid_t loc, const std::string &name) const { static_assert( diff --git a/spiner/spiner_types.hpp b/spiner/spiner_types.hpp index 43690e582..26d68763d 100644 --- a/spiner/spiner_types.hpp +++ b/spiner/spiner_types.hpp @@ -1,4 +1,4 @@ -// © (or copyright) 2019-2021. Triad National Security, LLC. All rights +// © (or copyright) 2019-2026. Triad National Security, LLC. All rights // reserved. This program was produced under U.S. Government contract // 89233218CNA000001 for Los Alamos National Laboratory (LANL), which is // operated by Triad National Security, LLC for the U.S. Department of @@ -19,4 +19,17 @@ #define H5_SUCCESS 0 #endif +namespace Spiner { +enum class DataStatus { + Empty = 0, + Unmanaged = 1, + AllocatedHost = 2, + AllocatedDevice = 3, + // indicates there is no management to do because all memory is + // static + Trivial = 4 +}; +enum class AllocationTarget { Host, Device }; +} // namespace Spiner + #endif // _SPINER_TYPES_HPP_ From c2c423881919098e7e2290433399ca71b2d343ae Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 14:24:53 -0400 Subject: [PATCH 03/17] databox looks sensible. Move allocation status to spiner types --- spiner/databox.hpp | 170 +++++++++++++++++++++++++++++++++------- spiner/spiner_types.hpp | 8 +- 2 files changed, 146 insertions(+), 32 deletions(-) diff --git a/spiner/databox.hpp b/spiner/databox.hpp index c52f4d284..bb8d95392 100644 --- a/spiner/databox.hpp +++ b/spiner/databox.hpp @@ -1,7 +1,7 @@ #ifndef _SPINER_DATABOX_HPP_ #define _SPINER_DATABOX_HPP_ //====================================================================== -// © (or copyright) 2019-2021. Triad National Security, LLC. All rights +// © (or copyright) 2019-2026. Triad National Security, LLC. All rights // reserved. This program was produced under U.S. Government contract // 89233218CNA000001 for Los Alamos National Laboratory (LANL), which is // operated by Triad National Security, LLC for the U.S. Department of @@ -15,7 +15,11 @@ // permit others to do so. //====================================================================== +// Generative AI was used to assist with modifications to this file. + #include +#include +#include #include #include #include @@ -33,6 +37,7 @@ #include "interpolation.hpp" #include "ports-of-call/portability.hpp" #include "ports-of-call/portable_arrays.hpp" +#include "ports-of-call/portable_errors.hpp" #include "sp5.hpp" #include "spiner_types.hpp" @@ -42,13 +47,6 @@ namespace Spiner { enum class IndexType { Interpolated = 0, Named = 1, Indexed = 2 }; -enum class DataStatus { - Empty = 0, - Unmanaged = 1, - AllocatedHost = 2, - AllocatedDevice = 3 -}; -enum class AllocationTarget { Host, Device }; template , typename Concept = @@ -89,7 +87,7 @@ class DataBox { inline DataBox(Args... args) noexcept : DataBox(AllocationTarget::Host, std::forward(args)...) {} - // Copy and move constructors. All shallow. + // Copies are shallow. Moves transfer ownership. inline DataBox(PortableMDArray A) noexcept : rank_(A.GetRank()), status_(DataStatus::Unmanaged), data_(A.data()), dataView_(A) { @@ -110,6 +108,30 @@ class DataBox { grids_[i] = src.grids_[i]; } } + // TODO(JMM): Do we really want to expose a move constructor? I + // think we do... + inline DataBox(DataBox &&src) noexcept + : rank_(src.rank_), status_(src.status_), data_(src.data_) { + setAllIndexed_(); + dataView_.InitWithShallowSlice(src.dataView_, 6, 0, src.dim(6)); + for (int i = 0; i < rank_; ++i) { + indices_[i] = src.indices_[i]; + } + // Only ACTUALLY move the data if we're managing it! + if (src.status_ != DataStatus::Unmanaged) { + for (int i = 0; i < rank_; ++i) { + // TODO(JMM): Should this be a move? (Yes probably) + grids_[i] = std::move(src.grids_[i]); + } + src.data_ = nullptr; + src.status_ = DataStatus::Empty; + src.rank_ = 0; + } else { + for (int i = 0; i < rank_; ++i) { + grids_[i] = src.grids_[i]; + } + } + } // Slice constructor PORTABLE_INLINE_FUNCTION @@ -231,12 +253,15 @@ class DataBox { // index, and N-1 is the slowest // TODO: is this intuitive? inline void setIndexType(int i, IndexType t) { - assert(0 <= i && i < rank_); + PORTABLE_REQUIRE(0 <= i && i < rank_, "Grid must be in index range"); indices_[i] = t; } inline void setRange(int i, Grid_t g) { + PORTABLE_REQUIRE(0 <= i && i < rank_, "Grid must be in index range"); + grids_[i].finalize(); // TODO(JMM): Do we want this? Probably. setIndexType(i, IndexType::Interpolated); - grids_[i] = g; + // TODO(JMM): Should this be a move? (Yes probably) + grids_[i] = std::move(g); } template inline void setRange(int i, Args &&...args) { @@ -267,14 +292,18 @@ class DataBox { } PORTABLE_INLINE_FUNCTION Grid_t &range(const int i) { return grids_[i]; } - // Assignment and move, both perform shallow copy + // Copy assignment is shallow; move assignment transfers ownership. PORTABLE_INLINE_FUNCTION DataBox & operator=(const DataBox &other); + inline DataBox & + operator=(DataBox &&other) noexcept; inline void copy(const DataBox &src); // utility info PORTABLE_INLINE_FUNCTION DataStatus dataStatus() const { return status_; } - PORTABLE_INLINE_FUNCTION bool isReference() { return status_ == DataStatus::Unmanaged; } + PORTABLE_INLINE_FUNCTION bool isReference() { + return status_ == DataStatus::Unmanaged; + } PORTABLE_INLINE_FUNCTION bool ownsAllocatedMemory() { return (status_ != DataStatus::Unmanaged); } @@ -309,11 +338,41 @@ class DataBox { return indices_[i]; } + // Binary serialization is intended only for transient communication between + // compatible Spiner builds. It is not a version-stable, architecture-stable, + // or persistent format; use HDF5 for persistent storage. + // + // Layout: + // [DataBox bytes][value data][grid dynamic memory 0]... + // [grid dynamic memory rank-1] + // // serialization routines // ------------------------------------ // this one reports size for serialize/deserialize + std::size_t dynamicMemorySizeInBytes() const { + std::size_t size = sizeBytes(); + for (int i = 0; i < rank_; ++i) { + size += grids_[i].dynamicMemorySizeInBytes(); + } + return size; + } + std::size_t serializedSizeInBytes() const { - return sizeBytes() + sizeof(*this); + return sizeof(*this) + dynamicMemorySizeInBytes(); + } + + std::size_t dumpDynamicMemory(char *dst) const { + PORTABLE_REQUIRE(status_ != DataStatus::AllocatedDevice, + "Dynamic memory cannot be dumped from device memory"); + std::size_t offst = 0; + if (sizeBytes() > 0) { // could also do data_ != nullptr + std::memcpy(dst, data_, sizeBytes()); + offst += sizeBytes(); + } + for (int i = 0; i < rank_; ++i) { + offst += grids_[i].dumpDynamicMemory(dst + offst); + } + return offst; } // this one takes the pointer `dst`, which is assumed to have // sufficient memory allocated, and fills it with the @@ -321,33 +380,34 @@ class DataBox { std::size_t serialize(char *dst) const { PORTABLE_REQUIRE(status_ != DataStatus::AllocatedDevice, "Serialization cannot be performed on device memory"); - memcpy(dst, this, sizeof(*this)); - std::size_t offst = sizeof(*this); - if (sizeBytes() > 0) { // could also do data_ != nullptr - memcpy(dst + offst, data_, sizeBytes()); - offst += sizeBytes(); - } - return offst; + std::memcpy(dst, this, sizeof(*this)); + return sizeof(*this) + dumpDynamicMemory(dst + sizeof(*this)); } // This sets the internal pointer based on a passed in src pointer, // which is assumed to be the right size. Used below in deSerialize // and may be used for serialization routines. Returns amount of src // pointer used. - std::size_t setPointer(T *src) { + std::size_t setPointer(char *src) { + std::size_t offst = 0; if (sizeBytes() > 0) { // could also do data_ != nullptr - data_ = src; + data_ = reinterpret_cast(src); // TODO(JMM): If portable arrays ever change maximum rank, this // line needs to change. dataView_.NewPortableMDArray(data_, dim(6), dim(5), dim(4), dim(3), dim(2), dim(1)); makeShallow(); + offst += sizeBytes(); } - return sizeBytes(); + for (int i = 0; i < rank_; ++i) { + offst += grids_[i].setPointer(src + offst); + } + return offst; + } + std::size_t setPointer(T *src) { + return setPointer(reinterpret_cast(src)); } - std::size_t setPointer(char *src) { return setPointer((T *)src); } - // This one takes a src pointer, which is assumed to contain a // databox and initializes the current databox. Note that the // databox becomes unmananged, as the contents of the box are still // the externally managed pointer. @@ -355,7 +415,12 @@ class DataBox { PORTABLE_REQUIRE( (status_ == DataStatus::Empty || status_ == DataStatus::Unmanaged), "Must not de-serialize into an active databox."); - memcpy(this, src, sizeof(*this)); + // TODO(JMM): This could be replaced by a per-grid warning as we + // do for databox data if we want. + for (int i = 0; i < rank_; ++i) { + grids_[i].finalize(); + } + std::memcpy(this, src, sizeof(*this)); // sanity check that de-serialization of trivially-copyable memory // was reasonable @@ -389,6 +454,9 @@ class DataBox { DataBox a{device_data, dim(6), dim(5), dim(4), dim(3), dim(2), dim(1)}; a.copyShape(*this); + for (int i = 0; i < rank_; ++i) { + a.grids_[i] = grids_[i].getOnDevice(); + } // set correct allocation status of the new databox // note this is ALWAYS device, even if host==device. a.status_ = DataStatus::AllocatedDevice; @@ -397,12 +465,18 @@ class DataBox { // TODO(JMM): Potentially use this for device-free void finalize() { - assert(ownsAllocatedMemory()); + PORTABLE_REQUIRE(ownsAllocatedMemory(), + "Must only finalize databox that owns its own data."); + // TODO(JMM): This may eventually need to be more complex + for (int i = 0; i < rank_; ++i) { + grids_[i].finalize(); + } if (status_ == DataStatus::AllocatedDevice) { PORTABLE_FREE(data_); } else if (status_ == DataStatus::AllocatedHost) { free(data_); } + data_ = nullptr; status_ = DataStatus::Empty; } @@ -925,6 +999,46 @@ DataBox::operator=(const DataBox &src) { return *this; } +// Move assignment transfers ownership of data and grid allocations. +template +inline DataBox &DataBox::operator=( + DataBox &&src) noexcept { + if (this != &src) { + // TODO(JMM): This worries me a little bit. Since databoxes and + // grids aren't reference counted, we have to be REALLY careful + // about move/copy semantics. + if (status_ == DataStatus::Unmanaged) { + for (int i = 0; i < rank_; ++i) { + grids_[i].finalize(); + } + } else { + finalize(); + } + + rank_ = src.rank_; + status_ = src.status_; + data_ = src.data_; + dataView_.InitWithShallowSlice(src.dataView_, 6, 0, src.dim(6)); + for (int i = 0; i < rank_; ++i) { + indices_[i] = src.indices_[i]; + } + + if (src.status_ != DataStatus::Unmanaged) { + for (int i = 0; i < rank_; ++i) { + grids_[i] = std::move(src.grids_[i]); + } + src.data_ = nullptr; + src.status_ = DataStatus::Empty; + src.rank_ = 0; + } else { + for (int i = 0; i < rank_; ++i) { + grids_[i] = src.grids_[i]; + } + } + } + return *this; +} + // Performs a deep copy template inline void diff --git a/spiner/spiner_types.hpp b/spiner/spiner_types.hpp index 26d68763d..5857d15ec 100644 --- a/spiner/spiner_types.hpp +++ b/spiner/spiner_types.hpp @@ -21,13 +21,13 @@ namespace Spiner { enum class DataStatus { + // indicates there is no management to do because all memory is + // static + Trivial = -1, Empty = 0, Unmanaged = 1, AllocatedHost = 2, - AllocatedDevice = 3, - // indicates there is no management to do because all memory is - // static - Trivial = 4 + AllocatedDevice = 3 }; enum class AllocationTarget { Host, Device }; } // namespace Spiner From 3e63fde2f3601ef1f88842ab9a9775b06a7850b5 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 14:25:39 -0400 Subject: [PATCH 04/17] missing static cast --- spiner/piecewise_grid_1d.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spiner/piecewise_grid_1d.hpp b/spiner/piecewise_grid_1d.hpp index 3a89d44f3..d5fa3ff2a 100644 --- a/spiner/piecewise_grid_1d.hpp +++ b/spiner/piecewise_grid_1d.hpp @@ -139,7 +139,7 @@ class PiecewiseGrid1D { } PORTABLE_INLINE_FUNCTION DataStatus dataStatus() const { - int status = DataStatus::Trivial; + int status = static_cast(DataStatus::Trivial); for (int i = 0; i < NGRIDS_; ++i) { int gs = static_cast(grids_[i].dataStatus()); PORTABLE_REQUIRE( From 4719ad107c1d1f48c1e0f6031e965e2edc399a75 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 14:26:30 -0400 Subject: [PATCH 05/17] update kokkos to 5.1 in test system --- test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 9e6f07c89..879820cb4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -42,7 +42,7 @@ if(SPINER_TEST_USE_KOKKOS) FetchContent_Declare( Kokkos GIT_REPOSITORY https://github.com/kokkos/kokkos.git - GIT_TAG 61d7db55fceac3318c987a291f77b844fd94c165 # TODO: make sure this is correct + GIT_TAG 5.1.0 # TODO: make sure this is correct ) foreach(ext_opt ${_spiner_content_opts}) From 5e7db5378f8f0adfff388f8be0b43c412b909cb4 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 14:58:39 -0400 Subject: [PATCH 06/17] concepts to make free and the deleter class easier to use --- spiner/databox.hpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/spiner/databox.hpp b/spiner/databox.hpp index bb8d95392..69638ed88 100644 --- a/spiner/databox.hpp +++ b/spiner/databox.hpp @@ -18,6 +18,7 @@ // Generative AI was used to assist with modifications to this file. #include +#include #include #include #include @@ -1014,7 +1015,7 @@ inline DataBox &DataBox::operator=( } else { finalize(); } - + rank_ = src.rank_; status_ = src.status_; data_ = src.data_; @@ -1112,13 +1113,19 @@ inline DataBox getOnDeviceDataBox(const DataBox &a_host) { return a_host.getOnDevice(); } -template -inline void free(DataBox &db) { - db.finalize(); + +template +concept Finalizable = requires(T &value) { + { value.finalize() } -> std::same_as; +}; + +template +inline void free(T &value) { + value.finalize(); } struct DBDeleter { - template + template void operator()(T *ptr) { ptr->finalize(); delete ptr; From 4c83c0bb2283bf9c66867e92c5673618625fa6ff Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:13:24 -0400 Subject: [PATCH 07/17] update docs --- doc/sphinx/src/databox.rst | 56 +++++++++++++++++++++++++------- doc/sphinx/src/interpolation.rst | 30 +++++++++++++++-- 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/doc/sphinx/src/databox.rst b/doc/sphinx/src/databox.rst index 47ed82371..6f852dca9 100644 --- a/doc/sphinx/src/databox.rst +++ b/doc/sphinx/src/databox.rst @@ -222,6 +222,12 @@ which returns ``true`` if a given databox is managing memory and returns ``false`` if the databox is managing memory and ``true`` otherwise. +.. warning:: + + A move operator and move constructor are each provided. However, + this is not the intended mechanism for interacting with + databoxes. Use with caution. + Using ``DataBox`` with smart pointers ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -274,6 +280,16 @@ function reports how much memory a ``DataBox`` object requires to be externally allocated. The function +.. cpp:function:: std::size_t DataBox::dynamicMemorySizeInBytes() const; + +reports the size of the value buffer and grid-owned dynamic memory, +excluding the inline ``DataBox`` object representation. The function + +.. cpp:function:: std::size_t DataBox::dumpDynamicMemory(char *dst) const; + +writes only that dynamic memory. It is used internally by ``serialize`` +after the inline object bytes have been copied. + .. cpp:function:: std::size_t serialize(char *dst) const; takes a ``char*`` pointer, assumed to contain enough space for a @@ -289,10 +305,10 @@ with the overload .. cpp:function:: std::size_t DataBox::setPointer(char *src); sets the underlying tabulated data from the src pointer, which is -assumed to be the right size and shape. This is useful for the -deSerialize function (described below) and for building your own -serialization/de-serialization routines in composite objects. The -function +assumed to be the right size and shape, and relocates the trailing +grid-owned dynamic memory. This is useful for the deSerialize +function (described below) and for building your own +serialization/de-serialization routines in composite objects. The function .. cpp:function:: std::size_t DataBox::deSerialize(char *src); @@ -307,6 +323,16 @@ contained in the ``src`` pointer. everything you want to do with the de-serialized ``DataBox`` is over. +The serialized memory is laid out as the inline ``DataBox`` metadata, +followed by the tabulated values and grid-owned dynamic memory. The +inline grid objects are already part of the ``DataBox`` metadata, so +their static bytes are not emitted a second time. Dynamic memory is +dumped for every dimension, including indexed and named dimensions, so +resource management does not depend on mutable interpolation metadata. + +The source buffer must remain alive and suitably aligned for the +entire lifetime of every ``DataBox`` de-serialized from it. + Putting this all together, an application of serialization/de-serialization probably looks like this: @@ -317,7 +343,7 @@ serialization/de-serialization probably looks like this: db.loadHDF(filename); // get size of databox - std::size_t allocate_size = db.serialSizeInBytes(); + std::size_t allocate_size = db.serializedSizeInBytes(); // Allocate the memory for the new databox. // In practice this would be an API call for, e.g., shared memory @@ -335,13 +361,17 @@ serialization/de-serialization probably looks like this: .. warning:: - The serialization routines described here are **not** architecture - aware. Serializing and de-serializing on a single architecture - inside a single executable will work fine. However, do not use - serialization as a file I/O strategy, as there is no guarantee that - the serialized format for a ``DataBox`` on one architecture will be - the same as on another. This is due to, for example, - architecture-specific differences in endianness and padding. + ``DataBox`` serialization is intended for transient communication + between processes using the same Spiner version and a compatible + architecture. The binary representation is not guaranteed to be + compatible across Spiner versions, architectures, compilers, or + build configurations. This is due to, for example, differences in + endianness, alignment, and padding. + + Do not use serialization as a persistent-storage or web-interchange + format. Use HDF5 for persistent, portable storage. Size calculation, + serialization, and de-serialization must all use a compatible Spiner + build. .. _`MPI Windows`: https://www.mpi-forum.org/docs/mpi-4.1/mpi41-report/node311.htm @@ -589,3 +619,5 @@ returns the total size of the underlying array in bytes. .. cpp:function:: int dim(int i) const; returns the size in a given dimension/direction, indexed from zero. + +Generative AI was used to assist with modifications to this page. diff --git a/doc/sphinx/src/interpolation.rst b/doc/sphinx/src/interpolation.rst index 5234065f1..f6e4b8d78 100644 --- a/doc/sphinx/src/interpolation.rst +++ b/doc/sphinx/src/interpolation.rst @@ -174,5 +174,31 @@ returns the number of points in the independent variable grid. Developer functionality ------------------------ -For developers, additional functionality is available. Please consult -the code. +All grid types implement the resource-management interface used by +``DataBox``: + +.. cpp:function:: std::size_t Grid::serializedSizeInBytes() const; +.. cpp:function:: std::size_t Grid::dynamicMemorySizeInBytes() const; +.. cpp:function:: std::size_t Grid::dumpDynamicMemory(char *dst) const; +.. cpp:function:: std::size_t Grid::serialize(char *dst) const; +.. cpp:function:: std::size_t Grid::deSerialize(char *src); +.. cpp:function:: std::size_t Grid::setPointer(char *src); +.. cpp:function:: Grid Grid::getOnDevice() const; +.. cpp:function:: void Grid::finalize(); + +These operations are currently trivial for ``RegularGrid1D`` because +it contains only inline data. ``PiecewiseGrid1D`` applies them +recursively to its component grids. This interface allows future grid +types to own dynamically allocated host or device data without +requiring grid-specific resource handling in ``DataBox``. + +``serialize`` writes the inline grid object followed by its dynamic +memory. ``dumpDynamicMemory`` writes only the latter, allowing a grid +embedded in a ``DataBox`` to avoid serializing its inline bytes twice. + +The binary serialization methods have the same transient, +build-dependent compatibility limitations as ``DataBox`` +serialization. See :ref:`the DataBox serialization documentation +` for details. + +Generative AI was used to assist with modifications to this page. From 13b278b69ed1209538ee47a801cd08f3bd4e5ae3 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:13:37 -0400 Subject: [PATCH 08/17] add tests for the owning grid objects --- test/test.cpp | 278 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 274 insertions(+), 4 deletions(-) diff --git a/test/test.cpp b/test/test.cpp index 9a4dffe89..5aa04993e 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -1,4 +1,4 @@ -// © (or copyright) 2019-2021. Triad National Security, LLC. All rights +// © (or copyright) 2019-2026. Triad National Security, LLC. All rights // reserved. This program was produced under U.S. Government contract // 89233218CNA000001 for Los Alamos National Laboratory (LANL), which is // operated by Triad National Security, LLC for the U.S. Department of @@ -11,9 +11,13 @@ // copies to the public, perform publicly and display publicly, and to // permit others to do so. +// Generative AI was used to assist with modifications to this file. + #include // std::min, std::max #include #include // sqrt +#include +#include #include #include #include @@ -41,6 +45,7 @@ template using PiecewiseGrid1D = Spiner::PiecewiseGrid1D; template using PiecewiseDB = Spiner::DataBox>; +using Spiner::DataStatus; PORTABLE_INLINE_FUNCTION Real linearFunction(Real z, Real y, Real x) { return x + y + z; @@ -100,9 +105,39 @@ TEST_CASE("RegularGrid1D", "[RegularGrid1D]") { REQUIRE(g.nPoints() == N); REQUIRE(g.dx() == (max - min) / ((Real)(N - 1))); } + + SECTION("A regular grid can be serialized and deserialized") { + RegularGrid1D grid(-1.0, 2.0, 7); + std::vector serialized(grid.serializedSizeInBytes()); + + const std::size_t written = grid.serialize(serialized.data()); + REQUIRE(written == serialized.size()); + + RegularGrid1D restored; + const std::size_t consumed = restored.deSerialize(serialized.data()); + REQUIRE(consumed == written); + REQUIRE(restored == grid); + REQUIRE(restored.setPointer(serialized.data()) == 0); + REQUIRE(restored.getOnDevice() == grid); + restored.finalize(); + grid.finalize(); + } } TEST_CASE("PiecewiseGrid1D", "[PiecewiseGrid1D]") { + SECTION("A default piecewise grid has a valid empty lifecycle") { + PiecewiseGrid1D<3> grid; + REQUIRE(grid.nGrids() == 0); + std::vector serialized(grid.serializedSizeInBytes()); + REQUIRE(grid.serialize(serialized.data()) == serialized.size()); + + PiecewiseGrid1D<3> restored; + REQUIRE(restored.deSerialize(serialized.data()) == serialized.size()); + REQUIRE(restored.nGrids() == 0); + restored.finalize(); + grid.finalize(); + } + GIVEN("Some regular grid 1Ds") { RegularGrid1D g1(0, 0.25, 3); RegularGrid1D g2(0.25, 0.75, 11); @@ -146,6 +181,23 @@ TEST_CASE("PiecewiseGrid1D", "[PiecewiseGrid1D]") { REQUIRE(std::abs(w[1] - 0.0024) < EPSTEST); REQUIRE(std::abs(w[0] - (1 - 0.0024)) < EPSTEST); } + AND_THEN("We can serialize and deserialize the nested grids") { + const std::size_t expected = sizeof(h) + h.dynamicMemorySizeInBytes(); + REQUIRE(h.serializedSizeInBytes() == expected); + std::vector serialized(expected); + REQUIRE(h.serialize(serialized.data()) == expected); + + PiecewiseGrid1D<3> restored; + REQUIRE(restored.deSerialize(serialized.data()) == expected); + REQUIRE(restored == h); + REQUIRE(restored.nPoints() == h.nPoints()); + REQUIRE(restored.index(0.8) == h.index(0.8)); + + auto device = h.getOnDevice(); + REQUIRE(device == h); + device.finalize(); + restored.finalize(); + } } } } @@ -616,8 +668,7 @@ TEST_CASE("DataBox Interpolation with piecewise grids", Real error = 0; portableReduce( "Interpolate 2D databox", 0, NFINE, 0, NFINE, - PORTABLE_LAMBDA(const int iy, const int ix, - Real &accumulate) { + PORTABLE_LAMBDA(const int iy, const int ix, Real &accumulate) { RegularGrid1D gfine(xmin, xmax, NFINE); Real x = gfine.x(ix); Real y = gfine.x(iy); @@ -672,7 +723,8 @@ SCENARIO("Serializing and deserializing a DataBox", } WHEN("We serialize the DataBox") { std::size_t serial_size = dbh.serializedSizeInBytes(); - REQUIRE(serial_size == (sizeof(dbh) + dbh.sizeBytes())); + REQUIRE(serial_size == (sizeof(dbh) + dbh.sizeBytes() + + RANK * g.dynamicMemorySizeInBytes())); char *db_serial = (char *)malloc(serial_size * sizeof(char)); std::size_t write_offst = dbh.serialize(db_serial); @@ -716,6 +768,13 @@ SCENARIO("Serializing and deserializing a DataBox", REQUIRE(dbh(i) == dbh2(i)); } } + + AND_THEN("The grid metadata is correct") { + for (int i = 0; i < RANK; ++i) { + REQUIRE(dbh2.indexType(i) == IndexType::Interpolated); + REQUIRE(dbh2.range(i) == dbh.range(i)); + } + } } // cleanup @@ -728,6 +787,217 @@ SCENARIO("Serializing and deserializing a DataBox", } } +/* A mocked up UniformGrid1D that owns an array of data. + * TODO(JMM): Remove/replace this once we have a NonuniformGrid1D. + * This is temporary! + */ +class OwningTestGrid1D { + public: + OwningTestGrid1D() = default; + OwningTestGrid1D(Real min, Real max, std::size_t n) + : n_(n), status_(DataStatus::AllocatedHost) { + points_ = static_cast(std::malloc(n_ * sizeof(Real))); + ++ownedAllocations; + const Real dx = (max - min) / static_cast(n_ - 1); + for (std::size_t i = 0; i < n_; ++i) { + points_[i] = min + static_cast(i) * dx; + } + } + PORTABLE_INLINE_FUNCTION OwningTestGrid1D(const OwningTestGrid1D &src) + : n_(src.n_), points_(src.points_), + status_(src.points_ == nullptr ? DataStatus::Empty + : DataStatus::Unmanaged) {} + PORTABLE_INLINE_FUNCTION OwningTestGrid1D & + operator=(const OwningTestGrid1D &src) { + if (this != &src) { + n_ = src.n_; + points_ = src.points_; + status_ = + src.points_ == nullptr ? DataStatus::Empty : DataStatus::Unmanaged; + } + return *this; + } + OwningTestGrid1D(OwningTestGrid1D &&src) noexcept { moveFrom_(src); } + OwningTestGrid1D &operator=(OwningTestGrid1D &&src) noexcept { + if (this != &src) { + finalize(); + moveFrom_(src); + } + return *this; + } + PORTABLE_INLINE_FUNCTION void weights(const Real x, int &ix, + Spiner::weights_t &w) const { + const Real inverseDx = + static_cast(n_ - 1) / (points_[n_ - 1] - points_[0]); + ix = static_cast(inverseDx * (x - points_[0])); + if (ix < 0) ix = 0; + if (ix >= static_cast(n_) - 1) ix = static_cast(n_) - 2; + w[1] = inverseDx * (x - points_[ix]); + w[0] = 1.0 - w[1]; + } + PORTABLE_INLINE_FUNCTION bool isWellFormed() const { + return points_ != nullptr && n_ > 1; + } + PORTABLE_INLINE_FUNCTION bool + operator==(const OwningTestGrid1D &other) const { + if (n_ != other.n_) { + return false; + } + for (std::size_t i = 0; i < n_; ++i) { + if (points_[i] != other.points_[i]) return false; + } + return true; + } + PORTABLE_INLINE_FUNCTION bool + operator!=(const OwningTestGrid1D &other) const { + return !(*this == other); + } + std::size_t dynamicMemorySizeInBytes() const { return n_ * sizeof(Real); } + std::size_t serializedSizeInBytes() const { + return sizeof(*this) + dynamicMemorySizeInBytes(); + } + std::size_t dumpDynamicMemory(char *dst) const { + std::memcpy(dst, points_, dynamicMemorySizeInBytes()); + return dynamicMemorySizeInBytes(); + } + std::size_t serialize(char *dst) const { + std::memcpy(dst, this, sizeof(*this)); + return sizeof(*this) + dumpDynamicMemory(dst + sizeof(*this)); + } + std::size_t setPointer(char *src) { + points_ = reinterpret_cast(src); + status_ = n_ == 0 ? DataStatus::Empty : DataStatus::Unmanaged; + return n_ * sizeof(Real); + } + std::size_t deSerialize(char *src) { + finalize(); + std::memcpy(this, src, sizeof(*this)); + return sizeof(*this) + setPointer(src + sizeof(*this)); + } + OwningTestGrid1D getOnDevice() const { + OwningTestGrid1D grid; + grid.n_ = n_; + if (n_ > 0) { + grid.points_ = static_cast(PORTABLE_MALLOC(n_ * sizeof(Real))); + portableCopyToDevice(grid.points_, points_, n_ * sizeof(Real)); + grid.status_ = DataStatus::AllocatedDevice; + ++ownedAllocations; + } + return grid; + } + void finalize() { + if (status_ == DataStatus::AllocatedHost) { + std::free(points_); + --ownedAllocations; + } else if (status_ == DataStatus::AllocatedDevice) { + PORTABLE_FREE(points_); + --ownedAllocations; + } + points_ = nullptr; + n_ = 0; + status_ = DataStatus::Empty; + } + PORTABLE_INLINE_FUNCTION Real *data() const { return points_; } + PORTABLE_INLINE_FUNCTION std::size_t nPoints() const { return n_; } + + // Track owned allocations to keep track of malloc/free calls. HOST + // only! + inline static int ownedAllocations = 0; + + private: + void moveFrom_(OwningTestGrid1D &src) { + n_ = src.n_; + points_ = src.points_; + status_ = src.status_; + if (src.status_ != DataStatus::Unmanaged) { + src.points_ = nullptr; + src.n_ = 0; + src.status_ = DataStatus::Empty; + } + } + std::size_t n_ = 0; + Real *points_ = nullptr; + DataStatus status_ = DataStatus::Empty; +}; + +TEST_CASE("DataBox delegates resource management to every grid", + "[DataBox][Serialize][GetOnDevice][OwningGrid]") { + using OwningDB = Spiner::DataBox; + constexpr int N = 4; + constexpr int RANK = 3; + + REQUIRE(OwningTestGrid1D::ownedAllocations == 0); + OwningDB db(N, N, N); + for (int i = 0; i < RANK; ++i) { + db.setRange(i, 0.0, 1.0, N); + } + REQUIRE(OwningTestGrid1D::ownedAllocations == RANK); + + for (int k = 0; k < N; ++k) { + for (int j = 0; j < N; ++j) { + for (int i = 0; i < N; ++i) { + db(k, j, i) = static_cast(i + j + k) / static_cast(N - 1); + } + } + } + + SECTION("Device copies deeply copy grid-owned data") { + OwningDB device; + device = db.getOnDevice(); + REQUIRE(OwningTestGrid1D::ownedAllocations == 2 * RANK); + for (int i = 0; i < RANK; ++i) { + REQUIRE(device.range(i).data() != db.range(i).data()); + } + + Real value = 0; + portableReduce( + "Interpolate with an owning grid", 0, 1, + PORTABLE_LAMBDA(const int, Real &result) { + result += device.interpToReal(0.5, 0.5, 0.5); + }, + value); + REQUIRE(std::abs(value - 1.5) <= EPSTEST); + + free(device); + REQUIRE(OwningTestGrid1D::ownedAllocations == RANK); + } + + SECTION("Every grid is serialized regardless of index type") { + db.setIndexType(1, IndexType::Indexed); + db.setIndexType(2, IndexType::Named); + + std::size_t expected = sizeof(db) + db.sizeBytes(); + for (int i = 0; i < RANK; ++i) { + expected += db.range(i).dynamicMemorySizeInBytes(); + } + REQUIRE(db.serializedSizeInBytes() == expected); + + std::vector serialized(expected); + REQUIRE(db.serialize(serialized.data()) == expected); + + OwningDB restored; + REQUIRE(restored.deSerialize(serialized.data()) == expected); + REQUIRE(restored.indexType(0) == IndexType::Interpolated); + REQUIRE(restored.indexType(1) == IndexType::Indexed); + REQUIRE(restored.indexType(2) == IndexType::Named); + + const char *begin = serialized.data(); + const char *end = begin + serialized.size(); + for (int i = 0; i < RANK; ++i) { + const char *gridData = + reinterpret_cast(restored.range(i).data()); + REQUIRE(gridData >= begin); + REQUIRE(gridData < end); + REQUIRE(restored.range(i) == db.range(i)); + restored.range(i).finalize(); + } + REQUIRE(OwningTestGrid1D::ownedAllocations == RANK); + } + + db.finalize(); + REQUIRE(OwningTestGrid1D::ownedAllocations == 0); +} + DataBox MakeFilledDB(int N, int &tot) { DataBox db(N, N, N); tot = 0; From ec1f2f938fc139025023c4bc32d3cabb7194b607 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:32:59 -0400 Subject: [PATCH 09/17] The move constructor was not a good idea. The AI misunderstood the pointer semantics --- doc/sphinx/src/databox.rst | 6 ---- spiner/databox.hpp | 66 ------------------------------------ spiner/piecewise_grid_1d.hpp | 23 ------------- test/test.cpp | 28 +++++++-------- 4 files changed, 12 insertions(+), 111 deletions(-) diff --git a/doc/sphinx/src/databox.rst b/doc/sphinx/src/databox.rst index 6f852dca9..9c2fe3feb 100644 --- a/doc/sphinx/src/databox.rst +++ b/doc/sphinx/src/databox.rst @@ -222,12 +222,6 @@ which returns ``true`` if a given databox is managing memory and returns ``false`` if the databox is managing memory and ``true`` otherwise. -.. warning:: - - A move operator and move constructor are each provided. However, - this is not the intended mechanism for interacting with - databoxes. Use with caution. - Using ``DataBox`` with smart pointers ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/spiner/databox.hpp b/spiner/databox.hpp index 69638ed88..a8081fb22 100644 --- a/spiner/databox.hpp +++ b/spiner/databox.hpp @@ -109,30 +109,6 @@ class DataBox { grids_[i] = src.grids_[i]; } } - // TODO(JMM): Do we really want to expose a move constructor? I - // think we do... - inline DataBox(DataBox &&src) noexcept - : rank_(src.rank_), status_(src.status_), data_(src.data_) { - setAllIndexed_(); - dataView_.InitWithShallowSlice(src.dataView_, 6, 0, src.dim(6)); - for (int i = 0; i < rank_; ++i) { - indices_[i] = src.indices_[i]; - } - // Only ACTUALLY move the data if we're managing it! - if (src.status_ != DataStatus::Unmanaged) { - for (int i = 0; i < rank_; ++i) { - // TODO(JMM): Should this be a move? (Yes probably) - grids_[i] = std::move(src.grids_[i]); - } - src.data_ = nullptr; - src.status_ = DataStatus::Empty; - src.rank_ = 0; - } else { - for (int i = 0; i < rank_; ++i) { - grids_[i] = src.grids_[i]; - } - } - } // Slice constructor PORTABLE_INLINE_FUNCTION @@ -296,8 +272,6 @@ class DataBox { // Copy assignment is shallow; move assignment transfers ownership. PORTABLE_INLINE_FUNCTION DataBox & operator=(const DataBox &other); - inline DataBox & - operator=(DataBox &&other) noexcept; inline void copy(const DataBox &src); // utility info @@ -1000,46 +974,6 @@ DataBox::operator=(const DataBox &src) { return *this; } -// Move assignment transfers ownership of data and grid allocations. -template -inline DataBox &DataBox::operator=( - DataBox &&src) noexcept { - if (this != &src) { - // TODO(JMM): This worries me a little bit. Since databoxes and - // grids aren't reference counted, we have to be REALLY careful - // about move/copy semantics. - if (status_ == DataStatus::Unmanaged) { - for (int i = 0; i < rank_; ++i) { - grids_[i].finalize(); - } - } else { - finalize(); - } - - rank_ = src.rank_; - status_ = src.status_; - data_ = src.data_; - dataView_.InitWithShallowSlice(src.dataView_, 6, 0, src.dim(6)); - for (int i = 0; i < rank_; ++i) { - indices_[i] = src.indices_[i]; - } - - if (src.status_ != DataStatus::Unmanaged) { - for (int i = 0; i < rank_; ++i) { - grids_[i] = std::move(src.grids_[i]); - } - src.data_ = nullptr; - src.status_ = DataStatus::Empty; - src.rank_ = 0; - } else { - for (int i = 0; i < rank_; ++i) { - grids_[i] = src.grids_[i]; - } - } - } - return *this; -} - // Performs a deep copy template inline void diff --git a/spiner/piecewise_grid_1d.hpp b/spiner/piecewise_grid_1d.hpp index d5fa3ff2a..7980ceed2 100644 --- a/spiner/piecewise_grid_1d.hpp +++ b/spiner/piecewise_grid_1d.hpp @@ -54,29 +54,6 @@ class PiecewiseGrid1D { PiecewiseGrid1D(const PiecewiseGrid1D &) = default; PORTABLE_INLINE_FUNCTION PiecewiseGrid1D & operator=(const PiecewiseGrid1D &) = default; - PiecewiseGrid1D(PiecewiseGrid1D &&other) noexcept : NGRIDS_(other.NGRIDS_) { - for (int i = 0; i < NGRIDS_; ++i) { - grids_[i] = std::move(other.grids_[i]); - pointTotals_[i] = other.pointTotals_[i]; - } - if (other.dataStatus() != DataStatus::Unmanaged) { - other.NGRIDS_ = 0; - } - } - PiecewiseGrid1D &operator=(PiecewiseGrid1D &&other) noexcept { - if (this != &other) { - finalize(); - NGRIDS_ = other.NGRIDS_; - for (int i = 0; i < NGRIDS_; ++i) { - grids_[i] = std::move(other.grids_[i]); - pointTotals_[i] = other.pointTotals_[i]; - } - if (other.dataStatus() != DataStatus::Unmanaged) { - other.NGRIDS_ = 0; - } - } - return *this; - } PiecewiseGrid1D(const std::vector> grids) { NGRIDS_ = grids.size(); PORTABLE_ALWAYS_REQUIRE( diff --git a/test/test.cpp b/test/test.cpp index 5aa04993e..329ba0578 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -805,26 +805,22 @@ class OwningTestGrid1D { } PORTABLE_INLINE_FUNCTION OwningTestGrid1D(const OwningTestGrid1D &src) : n_(src.n_), points_(src.points_), - status_(src.points_ == nullptr ? DataStatus::Empty - : DataStatus::Unmanaged) {} + status_(src.status_) {} PORTABLE_INLINE_FUNCTION OwningTestGrid1D & operator=(const OwningTestGrid1D &src) { - if (this != &src) { - n_ = src.n_; - points_ = src.points_; - status_ = - src.points_ == nullptr ? DataStatus::Empty : DataStatus::Unmanaged; - } - return *this; - } - OwningTestGrid1D(OwningTestGrid1D &&src) noexcept { moveFrom_(src); } - OwningTestGrid1D &operator=(OwningTestGrid1D &&src) noexcept { - if (this != &src) { - finalize(); - moveFrom_(src); - } + n_ = src.n_; + points_ = src.points_; + status_ = src.status_; return *this; } + //OwningTestGrid1D(OwningTestGrid1D &&src) noexcept { moveFrom_(src); } + //OwningTestGrid1D &operator=(OwningTestGrid1D &&src) noexcept { + // if (this != &src) { + // finalize(); + // moveFrom_(src); + // } + // return *this; + //} PORTABLE_INLINE_FUNCTION void weights(const Real x, int &ix, Spiner::weights_t &w) const { const Real inverseDx = From 1fd2471117923b631fd49e6042a888d2451b897b Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:35:25 -0400 Subject: [PATCH 10/17] cleanup tests a bit more --- test/test.cpp | 40 +++++++++++----------------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/test/test.cpp b/test/test.cpp index 329ba0578..24c52c1e0 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -797,7 +797,7 @@ class OwningTestGrid1D { OwningTestGrid1D(Real min, Real max, std::size_t n) : n_(n), status_(DataStatus::AllocatedHost) { points_ = static_cast(std::malloc(n_ * sizeof(Real))); - ++ownedAllocations; + ++owned_allocations; const Real dx = (max - min) / static_cast(n_ - 1); for (std::size_t i = 0; i < n_; ++i) { points_[i] = min + static_cast(i) * dx; @@ -813,14 +813,6 @@ class OwningTestGrid1D { status_ = src.status_; return *this; } - //OwningTestGrid1D(OwningTestGrid1D &&src) noexcept { moveFrom_(src); } - //OwningTestGrid1D &operator=(OwningTestGrid1D &&src) noexcept { - // if (this != &src) { - // finalize(); - // moveFrom_(src); - // } - // return *this; - //} PORTABLE_INLINE_FUNCTION void weights(const Real x, int &ix, Spiner::weights_t &w) const { const Real inverseDx = @@ -877,17 +869,17 @@ class OwningTestGrid1D { grid.points_ = static_cast(PORTABLE_MALLOC(n_ * sizeof(Real))); portableCopyToDevice(grid.points_, points_, n_ * sizeof(Real)); grid.status_ = DataStatus::AllocatedDevice; - ++ownedAllocations; + ++owned_allocations; } return grid; } void finalize() { if (status_ == DataStatus::AllocatedHost) { std::free(points_); - --ownedAllocations; + --owned_allocations; } else if (status_ == DataStatus::AllocatedDevice) { PORTABLE_FREE(points_); - --ownedAllocations; + --owned_allocations; } points_ = nullptr; n_ = 0; @@ -898,19 +890,9 @@ class OwningTestGrid1D { // Track owned allocations to keep track of malloc/free calls. HOST // only! - inline static int ownedAllocations = 0; + inline static int owned_allocations = 0; private: - void moveFrom_(OwningTestGrid1D &src) { - n_ = src.n_; - points_ = src.points_; - status_ = src.status_; - if (src.status_ != DataStatus::Unmanaged) { - src.points_ = nullptr; - src.n_ = 0; - src.status_ = DataStatus::Empty; - } - } std::size_t n_ = 0; Real *points_ = nullptr; DataStatus status_ = DataStatus::Empty; @@ -922,12 +904,12 @@ TEST_CASE("DataBox delegates resource management to every grid", constexpr int N = 4; constexpr int RANK = 3; - REQUIRE(OwningTestGrid1D::ownedAllocations == 0); + REQUIRE(OwningTestGrid1D::owned_allocations == 0); OwningDB db(N, N, N); for (int i = 0; i < RANK; ++i) { db.setRange(i, 0.0, 1.0, N); } - REQUIRE(OwningTestGrid1D::ownedAllocations == RANK); + REQUIRE(OwningTestGrid1D::owned_allocations == RANK); for (int k = 0; k < N; ++k) { for (int j = 0; j < N; ++j) { @@ -940,7 +922,7 @@ TEST_CASE("DataBox delegates resource management to every grid", SECTION("Device copies deeply copy grid-owned data") { OwningDB device; device = db.getOnDevice(); - REQUIRE(OwningTestGrid1D::ownedAllocations == 2 * RANK); + REQUIRE(OwningTestGrid1D::owned_allocations == 2 * RANK); for (int i = 0; i < RANK; ++i) { REQUIRE(device.range(i).data() != db.range(i).data()); } @@ -955,7 +937,7 @@ TEST_CASE("DataBox delegates resource management to every grid", REQUIRE(std::abs(value - 1.5) <= EPSTEST); free(device); - REQUIRE(OwningTestGrid1D::ownedAllocations == RANK); + REQUIRE(OwningTestGrid1D::owned_allocations == RANK); } SECTION("Every grid is serialized regardless of index type") { @@ -987,11 +969,11 @@ TEST_CASE("DataBox delegates resource management to every grid", REQUIRE(restored.range(i) == db.range(i)); restored.range(i).finalize(); } - REQUIRE(OwningTestGrid1D::ownedAllocations == RANK); + REQUIRE(OwningTestGrid1D::owned_allocations == RANK); } db.finalize(); - REQUIRE(OwningTestGrid1D::ownedAllocations == 0); + REQUIRE(OwningTestGrid1D::owned_allocations == 0); } DataBox MakeFilledDB(int N, int &tot) { From 99f0331b542cb43c3b882ea9843dcfec89bb2d4b Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:39:29 -0400 Subject: [PATCH 11/17] changelog --- .github/PULL_REQUEST_TEMPLATE.md | 1 + CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index ee055e205..092413280 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -16,6 +16,7 @@ detail. Why is this change required? What problem does it solve?--> - [ ] Code is formatted. (You can use the format_spiner make target.) - [ ] Adds a test for any bugs fixed. Adds tests for new features. - [ ] Document any new features, update documentation for changes made. +- [ ] Update the CHANGELOG.md file. - [ ] Make sure the copyright notice on any files you modified is up to date. - [ ] LANL employees: make sure tests pass both on the github CI and on the Darwin CI - [ ] If ML was used, make sure to add a disclaimer at the top of a file indicating ML was used to assist in generating the file. diff --git a/CHANGELOG.md b/CHANGELOG.md index 16a1c2c36..30643dc49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Current develop ### Added (new features/APIs/variables/...) +- [[MR 144]](https://github.com/lanl/spiner/pull/144) Add the ability for grid objects to own memory. This extends the public API but the real changes will come later. ### Fixed (Repair bugs, etc) - [[MR 141]](https://github.com/lanl/spiner/pull/142) fix issue with range() method on device From c1af0d3420d5be39d04020385494cb63ce91e57e Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:40:02 -0400 Subject: [PATCH 12/17] update plan history --- ...-lifecycle-plan.md => MR144-grid-resource-lifecycle-plan.md} | 2 ++ 1 file changed, 2 insertions(+) rename plan_histories/{260725-grid-resource-lifecycle-plan.md => MR144-grid-resource-lifecycle-plan.md} (99%) diff --git a/plan_histories/260725-grid-resource-lifecycle-plan.md b/plan_histories/MR144-grid-resource-lifecycle-plan.md similarity index 99% rename from plan_histories/260725-grid-resource-lifecycle-plan.md rename to plan_histories/MR144-grid-resource-lifecycle-plan.md index 8a803dfa7..82e233c5b 100644 --- a/plan_histories/260725-grid-resource-lifecycle-plan.md +++ b/plan_histories/MR144-grid-resource-lifecycle-plan.md @@ -340,3 +340,5 @@ implementation. - Device tests demonstrate that no grid retains a host-only pointer. - The documentation clearly disclaims cross-version and cross-architecture compatibility and recommends HDF5 for persistent storage. + +Generative AI (OpenAI Codex) was used to assist with modifications to this plan. From fead28e14e1b8908d7ae08a5d7d07f9f9f268a79 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:44:33 -0400 Subject: [PATCH 13/17] remove more move nonsense --- spiner/databox.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spiner/databox.hpp b/spiner/databox.hpp index a8081fb22..ebbb078f9 100644 --- a/spiner/databox.hpp +++ b/spiner/databox.hpp @@ -88,7 +88,7 @@ class DataBox { inline DataBox(Args... args) noexcept : DataBox(AllocationTarget::Host, std::forward(args)...) {} - // Copies are shallow. Moves transfer ownership. + // Copies are shallow. inline DataBox(PortableMDArray A) noexcept : rank_(A.GetRank()), status_(DataStatus::Unmanaged), data_(A.data()), dataView_(A) { @@ -235,10 +235,10 @@ class DataBox { } inline void setRange(int i, Grid_t g) { PORTABLE_REQUIRE(0 <= i && i < rank_, "Grid must be in index range"); - grids_[i].finalize(); // TODO(JMM): Do we want this? Probably. + grids_[i].finalize(); // TODO(JMM): Do we want this? setIndexType(i, IndexType::Interpolated); - // TODO(JMM): Should this be a move? (Yes probably) - grids_[i] = std::move(g); + // TODO(JMM): Should this be a move? + grids_[i] = g; } template inline void setRange(int i, Args &&...args) { From f6c69f614895c89405bfeee04ce6866bac047feb Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:45:59 -0400 Subject: [PATCH 14/17] oops accidentally deleted a line of comment --- spiner/databox.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/spiner/databox.hpp b/spiner/databox.hpp index ebbb078f9..b89d844c1 100644 --- a/spiner/databox.hpp +++ b/spiner/databox.hpp @@ -383,6 +383,7 @@ class DataBox { return setPointer(reinterpret_cast(src)); } + // This one takes a src pointer, which is assumed to contain a // databox and initializes the current databox. Note that the // databox becomes unmananged, as the contents of the box are still // the externally managed pointer. From eb3c56ca6db8c03e0004836cc0103f16870a39b5 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:46:46 -0400 Subject: [PATCH 15/17] remove more weird comments about move --- spiner/databox.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spiner/databox.hpp b/spiner/databox.hpp index b89d844c1..3955c0f3b 100644 --- a/spiner/databox.hpp +++ b/spiner/databox.hpp @@ -269,7 +269,7 @@ class DataBox { } PORTABLE_INLINE_FUNCTION Grid_t &range(const int i) { return grids_[i]; } - // Copy assignment is shallow; move assignment transfers ownership. + // Copy assignment is shallow PORTABLE_INLINE_FUNCTION DataBox & operator=(const DataBox &other); inline void copy(const DataBox &src); From 0321a3fe738e8d52318798b4533a204ace1843a1 Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:49:08 -0400 Subject: [PATCH 16/17] shoot need C++20 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ef770b344..acf8d2be2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -72,7 +72,7 @@ include(cmake/Format.cmake) # Add a library add_library(spiner INTERFACE) add_library(spiner::spiner ALIAS spiner) -target_compile_features(spiner INTERFACE cxx_std_17) +target_compile_features(spiner INTERFACE cxx_std_20) # ############################################################################## # Dependencies # From 476488bc59703cf51eb34087e67f83261723b89e Mon Sep 17 00:00:00 2001 From: Jonah Maxwell Miller Date: Sat, 25 Jul 2026 15:51:11 -0400 Subject: [PATCH 17/17] update test comment --- test/test.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/test.cpp b/test/test.cpp index 24c52c1e0..dc20b9714 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -788,8 +788,7 @@ SCENARIO("Serializing and deserializing a DataBox", } /* A mocked up UniformGrid1D that owns an array of data. - * TODO(JMM): Remove/replace this once we have a NonuniformGrid1D. - * This is temporary! + * TODO(JMM): Remove/replace/update this once we have a NonuniformGrid1D. */ class OwningTestGrid1D { public: @@ -804,8 +803,7 @@ class OwningTestGrid1D { } } PORTABLE_INLINE_FUNCTION OwningTestGrid1D(const OwningTestGrid1D &src) - : n_(src.n_), points_(src.points_), - status_(src.status_) {} + : n_(src.n_), points_(src.points_), status_(src.status_) {} PORTABLE_INLINE_FUNCTION OwningTestGrid1D & operator=(const OwningTestGrid1D &src) { n_ = src.n_;