From 953304e70fea92d41770c2d14d0a03870bfb3091 Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Fri, 17 Jul 2026 16:20:56 -0700 Subject: [PATCH 1/9] Implement state checkpointing for CollectionNode --- .../include/dwave-optimization/graph.hpp | 5 + .../dwave-optimization/nodes/collections.hpp | 7 + .../include/dwave-optimization/state.hpp | 16 ++ dwave/optimization/src/graph.cpp | 5 + dwave/optimization/src/nodes/collections.cpp | 154 +++++++++++++++- ...eature-checkpointing-b770d2f2b66f648d.yaml | 7 + tests/cpp/nodes/test_collections.cpp | 169 ++++++++++++++++++ 7 files changed, 362 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/feature-checkpointing-b770d2f2b66f648d.yaml diff --git a/dwave/optimization/include/dwave-optimization/graph.hpp b/dwave/optimization/include/dwave-optimization/graph.hpp index 22fbc3d6..b05baa0e 100644 --- a/dwave/optimization/include/dwave-optimization/graph.hpp +++ b/dwave/optimization/include/dwave-optimization/graph.hpp @@ -151,6 +151,11 @@ class Graph { std::function accept = [](const Graph&, State&) { return true; } ) const; + /// Propagate any pending changes to all nodes in the graph and commit them. + void propose(State& state) const; + // dev note: the name is a bit funny in this case, but we essentially want + // an overload for a "default" `sources` and `accept`. + /// Initialize the state of the given node and all predecessors recursively. static void recursive_initialize(State& state, const Node* ptr); /// Reset the state of the given node and all successors recursively. diff --git a/dwave/optimization/include/dwave-optimization/nodes/collections.hpp b/dwave/optimization/include/dwave-optimization/nodes/collections.hpp index 48b20119..68aeafae 100644 --- a/dwave/optimization/include/dwave-optimization/nodes/collections.hpp +++ b/dwave/optimization/include/dwave-optimization/nodes/collections.hpp @@ -32,8 +32,15 @@ class CollectionNode : public ArrayOutputMixin, public DecisionNode { // Set the node's state, tracking the diff. void assign(State& state, std::vector values) const; + /// Set the current state to match the one at the time the given checkpoint was created. + void assign_from_checkpoint(State& state, checkpoint_type& checkpoint) const; + void assign_from_checkpoint(State& state, checkpoint_type&& checkpoint) const; + const double* buff(const State& state) const override; + /// Get a checkpoint, an IOU that can be used to return the node to its current state. + checkpoint_type checkpoint(State& state) const; + void commit(State&) const override; std::span diff(const State& state) const override; diff --git a/dwave/optimization/include/dwave-optimization/state.hpp b/dwave/optimization/include/dwave-optimization/state.hpp index 884bf972..e82b9cbc 100644 --- a/dwave/optimization/include/dwave-optimization/state.hpp +++ b/dwave/optimization/include/dwave-optimization/state.hpp @@ -36,4 +36,20 @@ struct NodeStateData { using State = typename std::vector>; +/// A generic base class for node checkpoints. +struct NodeStateCheckpoint { + NodeStateCheckpoint() = default; + NodeStateCheckpoint(const NodeStateCheckpoint&) = default; + NodeStateCheckpoint(NodeStateCheckpoint&&) = delete; + NodeStateCheckpoint& operator=(const NodeStateCheckpoint&) = default; + NodeStateCheckpoint& operator=(NodeStateCheckpoint&&) = delete; + + virtual ~NodeStateCheckpoint() = default; + + /// Whether the checkpoint is still available to be used. + virtual bool valid() const = 0; +}; + +using checkpoint_type = std::unique_ptr; + } // namespace dwave::optimization diff --git a/dwave/optimization/src/graph.cpp b/dwave/optimization/src/graph.cpp index 733e94ae..a625cd40 100644 --- a/dwave/optimization/src/graph.cpp +++ b/dwave/optimization/src/graph.cpp @@ -195,6 +195,11 @@ void Graph::propose( } } +void Graph::propose(State& state) const { + propagate(state); + commit(state); +} + void Graph::recursive_initialize(State& state, const Node* ptr) { ssize_t index = ptr->topological_index(); diff --git a/dwave/optimization/src/nodes/collections.cpp b/dwave/optimization/src/nodes/collections.cpp index 0ea4f805..5dbb4d8b 100644 --- a/dwave/optimization/src/nodes/collections.cpp +++ b/dwave/optimization/src/nodes/collections.cpp @@ -14,9 +14,11 @@ #include "dwave-optimization/nodes/collections.hpp" +#include #include #include #include +#include namespace dwave::optimization { @@ -66,6 +68,51 @@ std::vector augment_collection_(std::vector values, const ssize_ return values; } +class CollectionStateData_; + +class CollectionCheckpoint_ : public NodeStateCheckpoint { + public: + CollectionCheckpoint_() = delete; + + CollectionCheckpoint_(CollectionStateData_* state_ptr); + + ~CollectionCheckpoint_() override; + + // detach the updates as a flattened view (in the forward order) + auto detach_updates() { + auto updates = std::move(updates_) | std::views::join; + assert(updates_.empty()); + return updates; + } + + ssize_t& drop() { return drop_; } + ssize_t drop() const { return drop_; } + + void emplace_updates(std::vector updates) { + if (drop_) { + // In C++23 we could use assign_range() which would be nicer + auto relevant = updates | std::views::drop(drop_); + updates_.emplace_back(relevant.begin(), relevant.end()); + drop_ = 0; + } else { + updates_.emplace_back(std::move(updates)); + } + } + + ssize_t size() { return size_; } + + bool valid() const override { return true; } + + private: + std::vector> updates_; + ssize_t drop_; + + ssize_t size_; + + CollectionCheckpoint_* older_checkpoint_ptr_; + std::variant newer_checkpoint_ptr_; +}; + class CollectionStateData_ : public NodeStateData { public: explicit CollectionStateData_(ssize_t n) : CollectionStateData_(n, n) {} @@ -109,11 +156,54 @@ class CollectionStateData_ : public NodeStateData { assert(this->size_ == size); } + void assign(std::unique_ptr& checkpoint) { + // convert the checkpoint into something we can read + auto* checkpoint_ptr = static_cast(checkpoint.get()); + + // Right now, you can only revert to the most recent checkpoint. It's + // pretty straightforward to support going further back, but this is all + // we need right now. + assert(older_checkpoint_ptr_ == checkpoint_ptr); + + // Ok, let's get ourselves to the same place as the checkpoint + + // we want to minimize the size of the visible buffer, so let's shrink ourselves + // if we need to + while (size_ > checkpoint_ptr->size()) shrink(); + + for (const auto& [idx, old, _] : checkpoint_ptr->detach_updates() | std::views::reverse) { + if (elements_[idx] == old) continue; // nothing to do + + all_updates_.emplace_back(idx, elements_[idx], old); + if (idx < size_) updates_.emplace_back(idx, elements_[idx], old); + + elements_[idx] = old; + } + + // now that we've filled in our buffer, grow until we're the correct size + while (size_ < checkpoint_ptr->size()) grow(); + + // update the "drop" value of the checkpoint so that our next commit doesn't + // add all of the changes we just added + checkpoint_ptr->drop() = all_updates_.size(); + } + const double* buff() const { return elements_.data(); } + std::unique_ptr checkpoint() { + return std::make_unique(this); + } + void commit() { updates_.clear(); - all_updates_.clear(); + + if (older_checkpoint_ptr_ != nullptr) { + older_checkpoint_ptr_->emplace_updates(std::move(all_updates_)); + assert(all_updates_.empty()); + } else { + all_updates_.clear(); + } + previous_size_ = size_; } @@ -213,8 +303,52 @@ class CollectionStateData_ : public NodeStateData { // commit/revert ssize_t size_; ssize_t previous_size_; + + friend CollectionCheckpoint_; + CollectionCheckpoint_* older_checkpoint_ptr_ = nullptr; }; +CollectionCheckpoint_::CollectionCheckpoint_(CollectionStateData_* state_ptr) : + updates_(), + drop_(state_ptr->all_updates_.size()), // so we ignore any updates added before we're made + size_(state_ptr->size()), + older_checkpoint_ptr_(state_ptr->older_checkpoint_ptr_), + newer_checkpoint_ptr_(state_ptr) { + if (older_checkpoint_ptr_ != nullptr) { + older_checkpoint_ptr_->newer_checkpoint_ptr_ = this; + + if (not state_ptr->all_updates_.empty()) { + older_checkpoint_ptr_->emplace_updates(state_ptr->all_updates_); // copy! + } + assert(older_checkpoint_ptr_->drop() == 0); + } + + std::get(newer_checkpoint_ptr_)->older_checkpoint_ptr_ = this; +} + +CollectionCheckpoint_::~CollectionCheckpoint_() { + if (older_checkpoint_ptr_ == nullptr) { + // We're the oldest checkpoint, so delete ourselves from the newer one + // and let any information we're holding die with us + std::visit([](auto* ptr) { ptr->older_checkpoint_ptr_ = nullptr; }, newer_checkpoint_ptr_); + } else { + // We're an intermediate checkpoint, so we need to pass any information we're + // holding to the next oldest checkpoint and update the pointers on either side of + // us + assert(older_checkpoint_ptr_->drop_ == 0); + for (std::vector& updates : updates_) { + older_checkpoint_ptr_->emplace_updates(std::move(updates)); + } + older_checkpoint_ptr_->drop_ = drop_; + + older_checkpoint_ptr_->newer_checkpoint_ptr_ = newer_checkpoint_ptr_; + std::visit( + [&](auto* ptr) { ptr->older_checkpoint_ptr_ = older_checkpoint_ptr_; }, + newer_checkpoint_ptr_ + ); + } +} + CollectionNode::CollectionNode(ssize_t max_value, ssize_t min_size, ssize_t max_size) : ArrayOutputMixin((min_size == max_size) ? max_size : Array::DYNAMIC_SIZE), max_value_(max_value), @@ -242,6 +376,24 @@ void CollectionNode::assign(State& state, std::vector values) const { data_ptr_(state)->assign(std::move(augemented), size); } +void CollectionNode::assign_from_checkpoint( + State& state, + std::unique_ptr& checkpoint +) const { + data_ptr_(state)->assign(checkpoint); +} +void CollectionNode::assign_from_checkpoint( + State& state, + std::unique_ptr&& checkpoint +) const { + assign_from_checkpoint(state, checkpoint); // call the lvalue version + checkpoint.reset(); +} + +std::unique_ptr CollectionNode::checkpoint(State& state) const { + return data_ptr_(state)->checkpoint(); +} + void CollectionNode::commit(State& state) const { data_ptr_(state)->commit(); } diff --git a/releasenotes/notes/feature-checkpointing-b770d2f2b66f648d.yaml b/releasenotes/notes/feature-checkpointing-b770d2f2b66f648d.yaml new file mode 100644 index 00000000..c82063e4 --- /dev/null +++ b/releasenotes/notes/feature-checkpointing-b770d2f2b66f648d.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Add a C++ ``Graph::propose(State&)`` overload that propagates and commits. + - | + Add checkpointing to ``CollectionNode``. + See `#510 `_. diff --git a/tests/cpp/nodes/test_collections.cpp b/tests/cpp/nodes/test_collections.cpp index 4090de04..218d0cff 100644 --- a/tests/cpp/nodes/test_collections.cpp +++ b/tests/cpp/nodes/test_collections.cpp @@ -703,6 +703,175 @@ TEST_CASE("SetNode") { } } } + + GIVEN("A set(5) initialized to {0, 1}") { + auto graph = Graph(); + + auto* set_ptr = graph.emplace_node(5); + + graph.emplace_node(set_ptr); + + auto state = graph.empty_state(); + set_ptr->initialize_state(state, {0, 1}); + graph.initialize_state(state); + + WHEN("We create a checkpoint from the initialized state") { + auto checkpoint0 = set_ptr->checkpoint(state); + + AND_WHEN("The set is changed to {3, 4, 1}") { + set_ptr->assign(state, {3, 4, 1}); + + graph.propose(state); + CHECK(set_ptr->size(state) == 3); + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + + AND_WHEN("We revert to the checkpoint") { + set_ptr->assign_from_checkpoint(state, checkpoint0); + graph.propose(state); + + THEN("The state has returned to {0, 1}") { + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + } + + AND_WHEN( + "The set is again mutated and then reverted using the same checkpoint" + ) { + set_ptr->assign(state, {4, 1, 0}); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1, 0})); + + set_ptr->assign_from_checkpoint(state, checkpoint0); + graph.propose(state); + + THEN("The state has returned to {0, 1}") { + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + } + } + } + + AND_WHEN("The set is changed to {3, 4, 1} and then the checkpoint is returned") { + set_ptr->assign(state, {3, 4, 1}); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint0)); + graph.propose(state); + + THEN("The state has returned to {0, 1} and the checkpoint is reset") { + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + CHECK(checkpoint0 == nullptr); + } + } + + AND_WHEN("We create another checkpoint") { + auto checkpoint1 = set_ptr->checkpoint(state); + + AND_WHEN("The set is changed to {4, 1}") { + set_ptr->assign(state, {4, 1}); + graph.propose(state); + + THEN("We can revert to the checkpoints one-by-one") { + set_ptr->assign_from_checkpoint(state, std::move(checkpoint1)); + graph.propose(state); + + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + + set_ptr->assign_from_checkpoint(state, checkpoint0); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + } + } + } + } + } + + WHEN("We mutate the state and then create a checkpoint before commiting") { + set_ptr->assign(state, {4, 1}); + auto checkpoint = set_ptr->checkpoint(state); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + + set_ptr->assign(state, {3, 4, 1}); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + + set_ptr->assign_from_checkpoint(state, checkpoint); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + } + + WHEN("We do several mutations and create several checkpoints within the same commit") { + auto checkpoint0 = set_ptr->checkpoint(state); + + set_ptr->assign(state, {4, 1}); + auto checkpoint1 = set_ptr->checkpoint(state); + + set_ptr->assign(state, {3, 4, 1}); + auto checkpoint2 = set_ptr->checkpoint(state); + + set_ptr->assign(state, {4, 2}); + graph.propose(state); // mix a propose in there + auto checkpoint3 = set_ptr->checkpoint(state); + + set_ptr->assign(state, {2}); + auto checkpoint4 = set_ptr->checkpoint(state); + + set_ptr->assign(state, {3, 2, 1, 0}); + graph.propose(state); + + THEN("we can go backwards through them without commiting and everything is correct") { + set_ptr->assign_from_checkpoint(state, std::move(checkpoint4)); + CHECK_THAT(set_ptr->view(state), RangeEquals({2})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint3)); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 2})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint2)); + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint1)); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint0)); + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + } + + THEN( + "we can go backwards through them and commit each time and everything is correct" + ) { + set_ptr->assign_from_checkpoint(state, std::move(checkpoint4)); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({2})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint3)); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 2})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint2)); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint1)); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint0)); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + } + + THEN("we can delete some intermediate checkpoints and everything stays valid") { + checkpoint2.reset(); + checkpoint4.reset(); + checkpoint0.reset(); + checkpoint3.reset(); + + set_ptr->assign_from_checkpoint(state, checkpoint1); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + } + } + } } } // namespace dwave::optimization From 214c5e23952649ba5adb24e8c74f3bcffe55e9e2 Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Mon, 20 Jul 2026 11:49:23 -0700 Subject: [PATCH 2/9] Fix reverts and dangling pointers for CollectionNode checkpoints --- .../include/dwave-optimization/array.hpp | 3 + dwave/optimization/src/nodes/collections.cpp | 75 +++++++-- tests/cpp/nodes/test_collections.cpp | 143 ++++++++++++++++-- 3 files changed, 196 insertions(+), 25 deletions(-) diff --git a/dwave/optimization/include/dwave-optimization/array.hpp b/dwave/optimization/include/dwave-optimization/array.hpp index 7182b3af..8ece49d1 100644 --- a/dwave/optimization/include/dwave-optimization/array.hpp +++ b/dwave/optimization/include/dwave-optimization/array.hpp @@ -347,6 +347,9 @@ struct Update { // Return true if the update does nothing - that is old and value are the same. bool identity() const { return null() || old == value; } + // Return the update that would undo the current update + Update inverse() const { return Update(index, value, old); } + // Use NaN to represent the "nothing" value used in placements/removals static constexpr double nothing = std::numeric_limits::signaling_NaN(); diff --git a/dwave/optimization/src/nodes/collections.cpp b/dwave/optimization/src/nodes/collections.cpp index 5dbb4d8b..0d168ab6 100644 --- a/dwave/optimization/src/nodes/collections.cpp +++ b/dwave/optimization/src/nodes/collections.cpp @@ -88,15 +88,36 @@ class CollectionCheckpoint_ : public NodeStateCheckpoint { ssize_t& drop() { return drop_; } ssize_t drop() const { return drop_; } - void emplace_updates(std::vector updates) { - if (drop_) { - // In C++23 we could use assign_range() which would be nicer - auto relevant = updates | std::views::drop(drop_); - updates_.emplace_back(relevant.begin(), relevant.end()); - drop_ = 0; - } else { + // Track the updates associated with a commit + void commit_updates(std::vector updates) { + assert(0 <= drop_ and static_cast(drop_) <= updates.size()); + + if (not drop_) { updates_.emplace_back(std::move(updates)); + return; } + + // Otherwise we only want to take the updates up to drop + // In C++23 we could use assign_range() which would be nicer + auto relevant = std::move(updates) | std::views::drop(drop_); + updates_.emplace_back(relevant.begin(), relevant.end()); + drop_ = 0; + } + + // Track the updates associated with a revert + void revert_updates(std::vector updates) { + assert(0 <= drop_ and static_cast(drop_) <= updates.size()); + + if (not drop_) return; // nothing to do + + // We want to track the updates that would revert the changes from the + // current state. + // In C++23 we could use assign_range() which would be nicer + auto relevant = std::move(updates) | std::views::take(drop_) | std::views::reverse | + std::views::transform([](const Update& up) { return up.inverse(); }); + updates_.emplace_back(relevant.begin(), relevant.end()); + + drop_ = 0; } ssize_t size() { return size_; } @@ -104,6 +125,8 @@ class CollectionCheckpoint_ : public NodeStateCheckpoint { bool valid() const override { return true; } private: + friend CollectionStateData_; + std::vector> updates_; ssize_t drop_; @@ -130,6 +153,15 @@ class CollectionStateData_ : public NodeStateData { assert(0 <= size_ and static_cast(size_) <= elements_.size()); } + ~CollectionStateData_() { + // make sure if we're destructed before the checkpoint that we clean + // up the dangling pointer + if (older_checkpoint_ptr_ != nullptr) { + older_checkpoint_ptr_->newer_checkpoint_ptr_ = + static_cast(nullptr); + } + } + void assign(std::vector&& values, ssize_t size) { // this should have been checked already by the CollectionNode assert(values.size() == elements_.size()); @@ -198,7 +230,7 @@ class CollectionStateData_ : public NodeStateData { updates_.clear(); if (older_checkpoint_ptr_ != nullptr) { - older_checkpoint_ptr_->emplace_updates(std::move(all_updates_)); + older_checkpoint_ptr_->commit_updates(std::move(all_updates_)); assert(all_updates_.empty()); } else { all_updates_.clear(); @@ -208,7 +240,9 @@ class CollectionStateData_ : public NodeStateData { } std::unique_ptr copy() const override { - return std::make_unique(*this); + auto uptr = std::make_unique(*this); + uptr->older_checkpoint_ptr_ = nullptr; // doesn't get to keep the checkpoints + return uptr; } std::span diff() const { return updates_; } @@ -240,16 +274,22 @@ class CollectionStateData_ : public NodeStateData { } void revert() { + updates_.clear(); + // Un-apply any changes by working backwards through all updates. // If we end up enforcing updates being sorted and unique later then // we could do this any order (or better in parallel). - for (const Update& update : all_updates_ | std::views::reverse) { elements_[update.index] = update.old; } - updates_.clear(); - all_updates_.clear(); + if (older_checkpoint_ptr_ != nullptr) { + older_checkpoint_ptr_->revert_updates(std::move(all_updates_)); + assert(all_updates_.empty()); + } else { + all_updates_.clear(); + } + size_ = previous_size_; } @@ -318,7 +358,7 @@ CollectionCheckpoint_::CollectionCheckpoint_(CollectionStateData_* state_ptr) : older_checkpoint_ptr_->newer_checkpoint_ptr_ = this; if (not state_ptr->all_updates_.empty()) { - older_checkpoint_ptr_->emplace_updates(state_ptr->all_updates_); // copy! + older_checkpoint_ptr_->commit_updates(state_ptr->all_updates_); // copy! } assert(older_checkpoint_ptr_->drop() == 0); } @@ -330,14 +370,19 @@ CollectionCheckpoint_::~CollectionCheckpoint_() { if (older_checkpoint_ptr_ == nullptr) { // We're the oldest checkpoint, so delete ourselves from the newer one // and let any information we're holding die with us - std::visit([](auto* ptr) { ptr->older_checkpoint_ptr_ = nullptr; }, newer_checkpoint_ptr_); + std::visit( + [](auto* ptr) { + if (ptr != nullptr) ptr->older_checkpoint_ptr_ = nullptr; + }, + newer_checkpoint_ptr_ + ); } else { // We're an intermediate checkpoint, so we need to pass any information we're // holding to the next oldest checkpoint and update the pointers on either side of // us assert(older_checkpoint_ptr_->drop_ == 0); for (std::vector& updates : updates_) { - older_checkpoint_ptr_->emplace_updates(std::move(updates)); + older_checkpoint_ptr_->commit_updates(std::move(updates)); } older_checkpoint_ptr_->drop_ = drop_; diff --git a/tests/cpp/nodes/test_collections.cpp b/tests/cpp/nodes/test_collections.cpp index 218d0cff..ed4818f6 100644 --- a/tests/cpp/nodes/test_collections.cpp +++ b/tests/cpp/nodes/test_collections.cpp @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include -#include #include -#include +#include #include "dwave-optimization/graph.hpp" #include "dwave-optimization/nodes/collections.hpp" @@ -783,21 +783,68 @@ TEST_CASE("SetNode") { } } } + + AND_WHEN("We destruct the state before the checkpoint") { + state = graph.empty_state(); + checkpoint0.reset(); + } } WHEN("We mutate the state and then create a checkpoint before commiting") { set_ptr->assign(state, {4, 1}); auto checkpoint = set_ptr->checkpoint(state); - graph.propose(state); - CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); - set_ptr->assign(state, {3, 4, 1}); - graph.propose(state); - CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + AND_WHEN("We do a sequence of commits") { + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); - set_ptr->assign_from_checkpoint(state, checkpoint); - graph.propose(state); - CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + set_ptr->assign(state, {3, 4, 1}); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 4, 1})); + + set_ptr->assign_from_checkpoint(state, checkpoint); + graph.propose(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + } + + AND_WHEN("We revert and then restore from the checkpoint") { + graph.propagate(state); + graph.revert(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint)); + graph.propagate(state); + AND_WHEN("we commit the change to the checkpoint") { + graph.commit(state); + + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + } + } + + AND_WHEN("We make more changes, save another checkpoint and then revert") { + set_ptr->assign(state, {3, 2, 1, 0}); + auto checkpoint1 = set_ptr->checkpoint(state); + + graph.propagate(state); + graph.revert(state); + CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1})); + + AND_WHEN("We revert to the first checkpoint") { + checkpoint1.reset(); // need to get rid of the second checkpoint first + + set_ptr->assign_from_checkpoint(state, std::move(checkpoint)); + graph.propose(state); + + CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); + } + + AND_WHEN("We revert to the second checkpoint") { + set_ptr->assign_from_checkpoint(state, std::move(checkpoint1)); + graph.propose(state); + + CHECK_THAT(set_ptr->view(state), RangeEquals({3, 2, 1, 0})); + } + } } WHEN("We do several mutations and create several checkpoints within the same commit") { @@ -870,6 +917,82 @@ TEST_CASE("SetNode") { graph.propose(state); CHECK_THAT(set_ptr->view(state), RangeEquals({4, 1})); } + + WHEN("We do some fuzzing with checkpoints") { + auto rng = std::default_random_engine(); + + // let's start by making a bunch of checkpoints with a copy of the buffer + // when the checkpoint was made + std::vector>> checkpoints; + { + // Randomly generate a state. There are more efficient ways + // to do this probably but for this test this is sufficient. + auto buffer = [&]() -> std::vector { + std::vector buff(5); + std::iota(buff.begin(), buff.end(), 0); + std::shuffle(buff.begin(), buff.end(), rng); + + std::uniform_int_distribution len(0, 4); + buff.erase(buff.begin() + len(rng), buff.end()); + + return buff; + }; + + // Commit anything that's pending + graph.propose(state); + + // Now, do a bunch of random actions + std::uniform_int_distribution action(0, 6); + for (int step = 0; step < 500; ++step) { + switch (action(rng)) { + case 0: + // make a checkpoint, tracking the current visible buffer + checkpoints.emplace_back( + set_ptr->checkpoint(state), + std::vector(set_ptr->begin(state), set_ptr->end(state)) + ); + break; + case 1: + // make a commit + graph.propagate(state); + graph.commit(state); + break; + case 2: + // make a revert + graph.propagate(state); + graph.revert(state); + break; + default: // we want to oversample this one + // assign a new state + set_ptr->assign(state, buffer()); + break; + } + } + + // Commit anything that's left over before the next step + graph.propose(state); + } + + // now, moving backwards through those checkpoints, let's randomly + // restore the state to the checkpoint or drop it + std::uniform_int_distribution flip(0, 1); + for (auto& [check, buff] : checkpoints | std::views::reverse) { + if (flip(rng)) { + set_ptr->assign_from_checkpoint(state, std::move(check)); + graph.propagate(state); + + CHECK_THAT(set_ptr->view(state), RangeEquals(buff)); + + if (flip(rng)) { + graph.commit(state); + } else { + graph.revert(state); + } + } else { + check.reset(); + } + } + } } } } From 9cd035b991321b3d67f390aabcca2df34247b414 Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Mon, 20 Jul 2026 12:14:21 -0700 Subject: [PATCH 3/9] Use offical Python images in CircleCI This avoids using GCC11 which had some bugs in their ranges implementation. --- .circleci/config.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 08b2bc79..343d55d7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -30,7 +30,7 @@ environment: &global-environment jobs: python-linux: docker: - - image: cimg/python:3.13 # just need a version that can install cibuildwheel + - image: cimg/python:3.13 # need a version that can install cibuildwheel and that has docker environment: <<: *global-environment @@ -57,7 +57,7 @@ jobs: python-linux-debug: docker: - - image: cimg/python:3.10 + - image: python:3.10 steps: - checkout @@ -127,7 +127,7 @@ jobs: python-sdist: docker: - - image: cimg/python:3.10 + - image: python:3.10 steps: - checkout @@ -239,7 +239,7 @@ jobs: serialization: docker: - - image: cimg/python:3.13 + - image: python:3.13 steps: - checkout @@ -282,7 +282,7 @@ jobs: docs: docker: - - image: cimg/python:3.13 # As of April 2026, the dwave-ocean-sdk uses 3.13 + - image: python:3.13 # As of April 2026, the dwave-ocean-sdk uses 3.13 steps: - checkout @@ -337,7 +337,7 @@ jobs: deploy: docker: - - image: cimg/python:3.10 + - image: python:3.10 steps: - attach_workspace: From 6ec49f916aff1150de913c046ac277f03c323b2c Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Mon, 20 Jul 2026 15:05:38 -0700 Subject: [PATCH 4/9] Add LinkedListCheckpoint helper class --- .../include/dwave-optimization/state.hpp | 4 +- dwave/optimization/src/nodes/_checkpoints.cpp | 44 +++++++ dwave/optimization/src/nodes/_checkpoints.hpp | 73 ++++++++++++ dwave/optimization/src/nodes/collections.cpp | 107 +++++------------- meson.build | 1 + 5 files changed, 151 insertions(+), 78 deletions(-) create mode 100644 dwave/optimization/src/nodes/_checkpoints.cpp create mode 100644 dwave/optimization/src/nodes/_checkpoints.hpp diff --git a/dwave/optimization/include/dwave-optimization/state.hpp b/dwave/optimization/include/dwave-optimization/state.hpp index e82b9cbc..a4a7e075 100644 --- a/dwave/optimization/include/dwave-optimization/state.hpp +++ b/dwave/optimization/include/dwave-optimization/state.hpp @@ -39,9 +39,9 @@ using State = typename std::vector>; /// A generic base class for node checkpoints. struct NodeStateCheckpoint { NodeStateCheckpoint() = default; - NodeStateCheckpoint(const NodeStateCheckpoint&) = default; + NodeStateCheckpoint(const NodeStateCheckpoint&) = delete; NodeStateCheckpoint(NodeStateCheckpoint&&) = delete; - NodeStateCheckpoint& operator=(const NodeStateCheckpoint&) = default; + NodeStateCheckpoint& operator=(const NodeStateCheckpoint&) = delete; NodeStateCheckpoint& operator=(NodeStateCheckpoint&&) = delete; virtual ~NodeStateCheckpoint() = default; diff --git a/dwave/optimization/src/nodes/_checkpoints.cpp b/dwave/optimization/src/nodes/_checkpoints.cpp new file mode 100644 index 00000000..77d2e95b --- /dev/null +++ b/dwave/optimization/src/nodes/_checkpoints.cpp @@ -0,0 +1,44 @@ +// Copyright 2026 D-Wave +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "_checkpoints.hpp" + +namespace dwave::optimization { + +// Place self between the state and any checkpoint it's currently holding +LinkedListCheckpoint::LinkedListCheckpoint(CheckpointableState& state) : + prev_ptr_(state.prev_ptr_), next_ptr_(&state) { + if (prev_ptr_ != nullptr) prev_ptr_->next_ptr_ = this; + state.prev_ptr_ = this; +} + +LinkedListCheckpoint::~LinkedListCheckpoint() { + if (prev_ptr_ != nullptr) prev_ptr_->next_ptr_ = next_ptr_; + + // Now make sure next_ptr is pointing to prev_ptr (which can be null) + std::visit( + [&](auto* next_ptr) -> void { + if (next_ptr == nullptr) return; // state was destructed first + next_ptr->prev_ptr_ = prev_ptr_; + }, + next_ptr_ + ); +} + +CheckpointableState::~CheckpointableState() { + if (prev_ptr_ == nullptr) return; // nothing to clean up + prev_ptr_->next_ptr_ = static_cast(nullptr); +} + +} // namespace dwave::optimization diff --git a/dwave/optimization/src/nodes/_checkpoints.hpp b/dwave/optimization/src/nodes/_checkpoints.hpp new file mode 100644 index 00000000..4f439daa --- /dev/null +++ b/dwave/optimization/src/nodes/_checkpoints.hpp @@ -0,0 +1,73 @@ +// Copyright 2026 D-Wave +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include + +#include "dwave-optimization/state.hpp" + +namespace dwave::optimization { + +class CheckpointableState; + +class LinkedListCheckpoint : public NodeStateCheckpoint { + public: + LinkedListCheckpoint() = delete; + // We're not moveable or copyable because NodeStateCheckpoint is not. + + LinkedListCheckpoint(CheckpointableState& state); + + ~LinkedListCheckpoint() override; + + protected: // todo: private? + friend CheckpointableState; + + LinkedListCheckpoint* prev_ptr_; + + // Is usually not nullptr unless the state has been destructed + std::variant next_ptr_; +}; + +class CheckpointableState : public NodeStateData { + public: + CheckpointableState() = default; + + CheckpointableState(const CheckpointableState& other) { + assert(false); + } + CheckpointableState(CheckpointableState&&) = default; + + CheckpointableState& operator=(const CheckpointableState&) { + assert(false); + } + CheckpointableState& operator=(CheckpointableState&&) = default; + + ~CheckpointableState(); + + protected: + template T> + T* checkpoint_ptr() { + return static_cast(prev_ptr_); + } + + private: // todo: private? + friend LinkedListCheckpoint; + + // The name is a bit confusing, but by making it match LinkedListCheckpoint::prev_ptr_ + // it makes the implementations of the various visit methods clearer. + LinkedListCheckpoint* prev_ptr_ = nullptr; // Will be nullptr if there are no checkpoints +}; + +} // namespace dwave::optimization diff --git a/dwave/optimization/src/nodes/collections.cpp b/dwave/optimization/src/nodes/collections.cpp index 0d168ab6..1d92bd49 100644 --- a/dwave/optimization/src/nodes/collections.cpp +++ b/dwave/optimization/src/nodes/collections.cpp @@ -18,7 +18,8 @@ #include #include #include -#include + +#include "_checkpoints.hpp" namespace dwave::optimization { @@ -70,13 +71,23 @@ std::vector augment_collection_(std::vector values, const ssize_ class CollectionStateData_; -class CollectionCheckpoint_ : public NodeStateCheckpoint { +class CollectionCheckpoint_ : public LinkedListCheckpoint { public: CollectionCheckpoint_() = delete; - CollectionCheckpoint_(CollectionStateData_* state_ptr); + CollectionCheckpoint_(CollectionStateData_& state); + + ~CollectionCheckpoint_() override { + // if we're the oldest checkpoint, just let whatever information we're + // holding get destructed with us + if (prev_ptr_ == nullptr) return; - ~CollectionCheckpoint_() override; + // otherwise we need to transfer our info over + auto* prev_ptr = static_cast(prev_ptr_); + assert(prev_ptr->drop_ == 0); + for (auto& updates : updates_) prev_ptr->commit_updates(std::move(updates)); + prev_ptr->drop_ = drop_; + } // detach the updates as a flattened view (in the forward order) auto detach_updates() { @@ -125,18 +136,13 @@ class CollectionCheckpoint_ : public NodeStateCheckpoint { bool valid() const override { return true; } private: - friend CollectionStateData_; - std::vector> updates_; ssize_t drop_; ssize_t size_; - - CollectionCheckpoint_* older_checkpoint_ptr_; - std::variant newer_checkpoint_ptr_; }; -class CollectionStateData_ : public NodeStateData { +class CollectionStateData_ : public CheckpointableState { public: explicit CollectionStateData_(ssize_t n) : CollectionStateData_(n, n) {} @@ -153,15 +159,6 @@ class CollectionStateData_ : public NodeStateData { assert(0 <= size_ and static_cast(size_) <= elements_.size()); } - ~CollectionStateData_() { - // make sure if we're destructed before the checkpoint that we clean - // up the dangling pointer - if (older_checkpoint_ptr_ != nullptr) { - older_checkpoint_ptr_->newer_checkpoint_ptr_ = - static_cast(nullptr); - } - } - void assign(std::vector&& values, ssize_t size) { // this should have been checked already by the CollectionNode assert(values.size() == elements_.size()); @@ -195,7 +192,7 @@ class CollectionStateData_ : public NodeStateData { // Right now, you can only revert to the most recent checkpoint. It's // pretty straightforward to support going further back, but this is all // we need right now. - assert(older_checkpoint_ptr_ == checkpoint_ptr); + assert(this->checkpoint_ptr() == checkpoint_ptr); // Ok, let's get ourselves to the same place as the checkpoint @@ -223,14 +220,14 @@ class CollectionStateData_ : public NodeStateData { const double* buff() const { return elements_.data(); } std::unique_ptr checkpoint() { - return std::make_unique(this); + return std::make_unique(*this); } void commit() { updates_.clear(); - if (older_checkpoint_ptr_ != nullptr) { - older_checkpoint_ptr_->commit_updates(std::move(all_updates_)); + if (auto* checkpoint_ptr = this->checkpoint_ptr()) { + checkpoint_ptr->commit_updates(std::move(all_updates_)); assert(all_updates_.empty()); } else { all_updates_.clear(); @@ -239,12 +236,6 @@ class CollectionStateData_ : public NodeStateData { previous_size_ = size_; } - std::unique_ptr copy() const override { - auto uptr = std::make_unique(*this); - uptr->older_checkpoint_ptr_ = nullptr; // doesn't get to keep the checkpoints - return uptr; - } - std::span diff() const { return updates_; } void exchange(ssize_t i, ssize_t j) { @@ -283,8 +274,8 @@ class CollectionStateData_ : public NodeStateData { elements_[update.index] = update.old; } - if (older_checkpoint_ptr_ != nullptr) { - older_checkpoint_ptr_->revert_updates(std::move(all_updates_)); + if (auto* checkpoint_ptr = this->checkpoint_ptr()) { + checkpoint_ptr->revert_updates(std::move(all_updates_)); assert(all_updates_.empty()); } else { all_updates_.clear(); @@ -329,6 +320,8 @@ class CollectionStateData_ : public NodeStateData { ssize_t size_diff() const { return size_ - previous_size_; } private: + friend CollectionCheckpoint_; + // The elements in the collection std::vector elements_; @@ -343,54 +336,16 @@ class CollectionStateData_ : public NodeStateData { // commit/revert ssize_t size_; ssize_t previous_size_; - - friend CollectionCheckpoint_; - CollectionCheckpoint_* older_checkpoint_ptr_ = nullptr; }; -CollectionCheckpoint_::CollectionCheckpoint_(CollectionStateData_* state_ptr) : +CollectionCheckpoint_::CollectionCheckpoint_(CollectionStateData_& state) : + LinkedListCheckpoint(state), updates_(), - drop_(state_ptr->all_updates_.size()), // so we ignore any updates added before we're made - size_(state_ptr->size()), - older_checkpoint_ptr_(state_ptr->older_checkpoint_ptr_), - newer_checkpoint_ptr_(state_ptr) { - if (older_checkpoint_ptr_ != nullptr) { - older_checkpoint_ptr_->newer_checkpoint_ptr_ = this; - - if (not state_ptr->all_updates_.empty()) { - older_checkpoint_ptr_->commit_updates(state_ptr->all_updates_); // copy! - } - assert(older_checkpoint_ptr_->drop() == 0); - } - - std::get(newer_checkpoint_ptr_)->older_checkpoint_ptr_ = this; -} - -CollectionCheckpoint_::~CollectionCheckpoint_() { - if (older_checkpoint_ptr_ == nullptr) { - // We're the oldest checkpoint, so delete ourselves from the newer one - // and let any information we're holding die with us - std::visit( - [](auto* ptr) { - if (ptr != nullptr) ptr->older_checkpoint_ptr_ = nullptr; - }, - newer_checkpoint_ptr_ - ); - } else { - // We're an intermediate checkpoint, so we need to pass any information we're - // holding to the next oldest checkpoint and update the pointers on either side of - // us - assert(older_checkpoint_ptr_->drop_ == 0); - for (std::vector& updates : updates_) { - older_checkpoint_ptr_->commit_updates(std::move(updates)); - } - older_checkpoint_ptr_->drop_ = drop_; - - older_checkpoint_ptr_->newer_checkpoint_ptr_ = newer_checkpoint_ptr_; - std::visit( - [&](auto* ptr) { ptr->older_checkpoint_ptr_ = older_checkpoint_ptr_; }, - newer_checkpoint_ptr_ - ); + drop_(state.all_updates_.size()), // so we ignore any updates added before we're made + size_(state.size()) { + if (auto* prev_checkpoint = static_cast(prev_ptr_)) { + prev_checkpoint->commit_updates(state.all_updates_); + assert(prev_checkpoint->drop() == 0); } } diff --git a/meson.build b/meson.build index 690d7165..b8882ec8 100644 --- a/meson.build +++ b/meson.build @@ -27,6 +27,7 @@ py = import('python').find_installation(pure: false) dwave_optimization_include = include_directories('dwave/optimization/include/') dwave_optimization_src = [ + 'dwave/optimization/src/nodes/_checkpoints.cpp', 'dwave/optimization/src/nodes/binaryop.cpp', 'dwave/optimization/src/nodes/collections.cpp', 'dwave/optimization/src/nodes/constants.cpp', From fc44824cba614127daaf23e057324698e50d975e Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Mon, 20 Jul 2026 15:15:16 -0700 Subject: [PATCH 5/9] Fix the copying of checkpointed states --- dwave/optimization/src/nodes/_checkpoints.hpp | 8 ++------ dwave/optimization/src/nodes/collections.cpp | 4 ++++ tests/cpp/nodes/test_collections.cpp | 6 ++++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/dwave/optimization/src/nodes/_checkpoints.hpp b/dwave/optimization/src/nodes/_checkpoints.hpp index 4f439daa..3753b78b 100644 --- a/dwave/optimization/src/nodes/_checkpoints.hpp +++ b/dwave/optimization/src/nodes/_checkpoints.hpp @@ -44,14 +44,10 @@ class CheckpointableState : public NodeStateData { public: CheckpointableState() = default; - CheckpointableState(const CheckpointableState& other) { - assert(false); - } + CheckpointableState(const CheckpointableState&) {} // the checkpoint pointer is not copied CheckpointableState(CheckpointableState&&) = default; - CheckpointableState& operator=(const CheckpointableState&) { - assert(false); - } + CheckpointableState& operator=(const CheckpointableState&) = delete; CheckpointableState& operator=(CheckpointableState&&) = default; ~CheckpointableState(); diff --git a/dwave/optimization/src/nodes/collections.cpp b/dwave/optimization/src/nodes/collections.cpp index 1d92bd49..3f7e6c72 100644 --- a/dwave/optimization/src/nodes/collections.cpp +++ b/dwave/optimization/src/nodes/collections.cpp @@ -236,6 +236,10 @@ class CollectionStateData_ : public CheckpointableState { previous_size_ = size_; } + std::unique_ptr copy() const override { + return std::make_unique(*this); + } + std::span diff() const { return updates_; } void exchange(ssize_t i, ssize_t j) { diff --git a/tests/cpp/nodes/test_collections.cpp b/tests/cpp/nodes/test_collections.cpp index ed4818f6..c8ac10d0 100644 --- a/tests/cpp/nodes/test_collections.cpp +++ b/tests/cpp/nodes/test_collections.cpp @@ -788,6 +788,12 @@ TEST_CASE("SetNode") { state = graph.empty_state(); checkpoint0.reset(); } + + THEN("We can copy the state") { + auto cp = state[0]->copy(); + // this is a smoke test because there is no public way to check + // that the checkpoint didn't get copied over + } } WHEN("We mutate the state and then create a checkpoint before commiting") { From f711e63900c1cc1a5ae897011928d6cef448a85e Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Mon, 20 Jul 2026 15:25:51 -0700 Subject: [PATCH 6/9] Drop unused NodeStateCheckpoint::valid() method --- dwave/optimization/include/dwave-optimization/state.hpp | 3 --- dwave/optimization/src/nodes/collections.cpp | 2 -- 2 files changed, 5 deletions(-) diff --git a/dwave/optimization/include/dwave-optimization/state.hpp b/dwave/optimization/include/dwave-optimization/state.hpp index a4a7e075..4b270db9 100644 --- a/dwave/optimization/include/dwave-optimization/state.hpp +++ b/dwave/optimization/include/dwave-optimization/state.hpp @@ -45,9 +45,6 @@ struct NodeStateCheckpoint { NodeStateCheckpoint& operator=(NodeStateCheckpoint&&) = delete; virtual ~NodeStateCheckpoint() = default; - - /// Whether the checkpoint is still available to be used. - virtual bool valid() const = 0; }; using checkpoint_type = std::unique_ptr; diff --git a/dwave/optimization/src/nodes/collections.cpp b/dwave/optimization/src/nodes/collections.cpp index 3f7e6c72..79f2afd6 100644 --- a/dwave/optimization/src/nodes/collections.cpp +++ b/dwave/optimization/src/nodes/collections.cpp @@ -133,8 +133,6 @@ class CollectionCheckpoint_ : public LinkedListCheckpoint { ssize_t size() { return size_; } - bool valid() const override { return true; } - private: std::vector> updates_; ssize_t drop_; From e85f85f2339c3c3a82810b44c2734c9e62fbf98c Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Tue, 21 Jul 2026 13:01:50 -0700 Subject: [PATCH 7/9] Add DiffCheckpoint class for checkpoints that track diffs --- dwave/optimization/src/nodes/_checkpoints.cpp | 52 ++++++++++++- dwave/optimization/src/nodes/_checkpoints.hpp | 62 ++++++++++----- dwave/optimization/src/nodes/_state.hpp | 16 ++++ dwave/optimization/src/nodes/collections.cpp | 75 +------------------ 4 files changed, 113 insertions(+), 92 deletions(-) diff --git a/dwave/optimization/src/nodes/_checkpoints.cpp b/dwave/optimization/src/nodes/_checkpoints.cpp index 77d2e95b..60a4d8bc 100644 --- a/dwave/optimization/src/nodes/_checkpoints.cpp +++ b/dwave/optimization/src/nodes/_checkpoints.cpp @@ -16,6 +16,11 @@ namespace dwave::optimization { +CheckpointableState::~CheckpointableState() { + if (prev_ptr_ == nullptr) return; // nothing to clean up + prev_ptr_->next_ptr_ = static_cast(nullptr); +} + // Place self between the state and any checkpoint it's currently holding LinkedListCheckpoint::LinkedListCheckpoint(CheckpointableState& state) : prev_ptr_(state.prev_ptr_), next_ptr_(&state) { @@ -36,9 +41,50 @@ LinkedListCheckpoint::~LinkedListCheckpoint() { ); } -CheckpointableState::~CheckpointableState() { - if (prev_ptr_ == nullptr) return; // nothing to clean up - prev_ptr_->next_ptr_ = static_cast(nullptr); +DiffCheckpoint::DiffCheckpoint(CheckpointableState& state, std::span diff) : + LinkedListCheckpoint(state), updates_(), drop_(diff.size()) { + if (auto* prev_ptr = static_cast(prev_ptr_)) { + prev_ptr->commit_updates(std::vector(diff.begin(), diff.end())); + assert(prev_ptr->drop_ == 0); + } +} + +DiffCheckpoint::~DiffCheckpoint() { + // if we're the oldest checkpoint, just let whatever information we're + // holding get destructed with us + if (prev_ptr_ == nullptr) return; + + // otherwise we need to transfer our info over + auto* prev_ptr = static_cast(prev_ptr_); + assert(prev_ptr->drop_ == 0); + for (auto& updates : updates_) prev_ptr->commit_updates(std::move(updates)); + prev_ptr->drop_ = drop_; +} + +void DiffCheckpoint::commit_updates(std::vector updates) { + assert(0 <= drop_ and static_cast(drop_) <= updates.size()); + + if (drop_) { + updates.erase(updates.begin(), updates.begin() + drop_); + drop_ = 0; + } + + updates_.emplace_back(std::move(updates)); +} + +void DiffCheckpoint::revert_updates(std::vector updates) { + assert(0 <= drop_ and static_cast(drop_) <= updates.size()); + + if (not drop_) return; // nothing to do + + // We want to track the updates that would revert the changes from the + // current state. + // In C++23 we could use assign_range() which would be nicer + auto relevant = std::move(updates) | std::views::take(drop_) | std::views::reverse | + std::views::transform([](const Update& up) { return up.inverse(); }); + updates_.emplace_back(relevant.begin(), relevant.end()); + + drop_ = 0; } } // namespace dwave::optimization diff --git a/dwave/optimization/src/nodes/_checkpoints.hpp b/dwave/optimization/src/nodes/_checkpoints.hpp index 3753b78b..39ac72db 100644 --- a/dwave/optimization/src/nodes/_checkpoints.hpp +++ b/dwave/optimization/src/nodes/_checkpoints.hpp @@ -14,13 +14,42 @@ #pragma once +#include #include +#include +#include "dwave-optimization/array.hpp" #include "dwave-optimization/state.hpp" namespace dwave::optimization { -class CheckpointableState; +class LinkedListCheckpoint; + +class CheckpointableState { + public: + CheckpointableState() = default; + + CheckpointableState(const CheckpointableState&) {} // the checkpoint pointer is not copied + CheckpointableState(CheckpointableState&&) = default; + + CheckpointableState& operator=(const CheckpointableState&) = delete; + CheckpointableState& operator=(CheckpointableState&&) = default; + + ~CheckpointableState(); + + protected: + template T> + T* checkpoint_ptr() { + return static_cast(prev_ptr_); + } + + private: // todo: private? + friend LinkedListCheckpoint; + + // The name is a bit confusing, but by making it match LinkedListCheckpoint::prev_ptr_ + // it makes the implementations of the various visit methods clearer. + LinkedListCheckpoint* prev_ptr_ = nullptr; // Will be nullptr if there are no checkpoints +}; class LinkedListCheckpoint : public NodeStateCheckpoint { public: @@ -40,30 +69,27 @@ class LinkedListCheckpoint : public NodeStateCheckpoint { std::variant next_ptr_; }; -class CheckpointableState : public NodeStateData { +class DiffCheckpoint : public LinkedListCheckpoint { public: - CheckpointableState() = default; + DiffCheckpoint(CheckpointableState& state, std::span diff); - CheckpointableState(const CheckpointableState&) {} // the checkpoint pointer is not copied - CheckpointableState(CheckpointableState&&) = default; + ~DiffCheckpoint() override; - CheckpointableState& operator=(const CheckpointableState&) = delete; - CheckpointableState& operator=(CheckpointableState&&) = default; - - ~CheckpointableState(); + void commit_updates(std::vector updates); - protected: - template T> - T* checkpoint_ptr() { - return static_cast(prev_ptr_); + auto detach_updates() { + auto updates = std::move(updates_) | std::views::join; + assert(updates_.empty()); + return updates; } - private: // todo: private? - friend LinkedListCheckpoint; + ssize_t& drop() { return drop_; } - // The name is a bit confusing, but by making it match LinkedListCheckpoint::prev_ptr_ - // it makes the implementations of the various visit methods clearer. - LinkedListCheckpoint* prev_ptr_ = nullptr; // Will be nullptr if there are no checkpoints + void revert_updates(std::vector updates); + + private: + std::vector> updates_; + ssize_t drop_; }; } // namespace dwave::optimization diff --git a/dwave/optimization/src/nodes/_state.hpp b/dwave/optimization/src/nodes/_state.hpp index bc715202..5f4c8162 100644 --- a/dwave/optimization/src/nodes/_state.hpp +++ b/dwave/optimization/src/nodes/_state.hpp @@ -91,6 +91,22 @@ class ArrayStateData { assert(size_ >= 0 && static_cast(size_) == buffer.size()); } + // Commit the changes and clear the diff by returning the diff buffer. + std::vector commit_and_detach() { + std::vector tmp; + std::swap(updates, tmp); + // AlexC: we could now do updates.reserve(tmp.size()) under the assumption + // that future update buffers will be a similar size. On the other hand, + // not doing this provides another meaningful difference to ::commit(). + // For now, I think it make sense to not but performance testing needed. + + previous_size_ = buffer.size(); + assert(size_ >= 0 && static_cast(size_) == buffer.size()); + + assert(updates.empty()); + return tmp; + } + std::span diff() const noexcept { return updates; } // Append a new value to the buffer, tracking the addition in the diff diff --git a/dwave/optimization/src/nodes/collections.cpp b/dwave/optimization/src/nodes/collections.cpp index 79f2afd6..fb265ed6 100644 --- a/dwave/optimization/src/nodes/collections.cpp +++ b/dwave/optimization/src/nodes/collections.cpp @@ -71,76 +71,17 @@ std::vector augment_collection_(std::vector values, const ssize_ class CollectionStateData_; -class CollectionCheckpoint_ : public LinkedListCheckpoint { +class CollectionCheckpoint_ : public DiffCheckpoint { public: - CollectionCheckpoint_() = delete; - CollectionCheckpoint_(CollectionStateData_& state); - ~CollectionCheckpoint_() override { - // if we're the oldest checkpoint, just let whatever information we're - // holding get destructed with us - if (prev_ptr_ == nullptr) return; - - // otherwise we need to transfer our info over - auto* prev_ptr = static_cast(prev_ptr_); - assert(prev_ptr->drop_ == 0); - for (auto& updates : updates_) prev_ptr->commit_updates(std::move(updates)); - prev_ptr->drop_ = drop_; - } - - // detach the updates as a flattened view (in the forward order) - auto detach_updates() { - auto updates = std::move(updates_) | std::views::join; - assert(updates_.empty()); - return updates; - } - - ssize_t& drop() { return drop_; } - ssize_t drop() const { return drop_; } - - // Track the updates associated with a commit - void commit_updates(std::vector updates) { - assert(0 <= drop_ and static_cast(drop_) <= updates.size()); - - if (not drop_) { - updates_.emplace_back(std::move(updates)); - return; - } - - // Otherwise we only want to take the updates up to drop - // In C++23 we could use assign_range() which would be nicer - auto relevant = std::move(updates) | std::views::drop(drop_); - updates_.emplace_back(relevant.begin(), relevant.end()); - drop_ = 0; - } - - // Track the updates associated with a revert - void revert_updates(std::vector updates) { - assert(0 <= drop_ and static_cast(drop_) <= updates.size()); - - if (not drop_) return; // nothing to do - - // We want to track the updates that would revert the changes from the - // current state. - // In C++23 we could use assign_range() which would be nicer - auto relevant = std::move(updates) | std::views::take(drop_) | std::views::reverse | - std::views::transform([](const Update& up) { return up.inverse(); }); - updates_.emplace_back(relevant.begin(), relevant.end()); - - drop_ = 0; - } - - ssize_t size() { return size_; } + ssize_t size() const { return size_; } private: - std::vector> updates_; - ssize_t drop_; - ssize_t size_; }; -class CollectionStateData_ : public CheckpointableState { +class CollectionStateData_ : public NodeStateData, public CheckpointableState { public: explicit CollectionStateData_(ssize_t n) : CollectionStateData_(n, n) {} @@ -341,15 +282,7 @@ class CollectionStateData_ : public CheckpointableState { }; CollectionCheckpoint_::CollectionCheckpoint_(CollectionStateData_& state) : - LinkedListCheckpoint(state), - updates_(), - drop_(state.all_updates_.size()), // so we ignore any updates added before we're made - size_(state.size()) { - if (auto* prev_checkpoint = static_cast(prev_ptr_)) { - prev_checkpoint->commit_updates(state.all_updates_); - assert(prev_checkpoint->drop() == 0); - } -} + DiffCheckpoint(state, state.all_updates_), size_(state.size()) {} CollectionNode::CollectionNode(ssize_t max_value, ssize_t min_size, ssize_t max_size) : ArrayOutputMixin((min_size == max_size) ? max_size : Array::DYNAMIC_SIZE), From f70699e837c57a82d1b435c1e2489abbda66f4a1 Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Tue, 21 Jul 2026 14:16:23 -0700 Subject: [PATCH 8/9] Go back to using CircleCI image for some CI tasks --- .circleci/config.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 343d55d7..b66c34b2 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -239,7 +239,7 @@ jobs: serialization: docker: - - image: python:3.13 + - image: cimg/python:3.13 steps: - checkout @@ -282,7 +282,7 @@ jobs: docs: docker: - - image: python:3.13 # As of April 2026, the dwave-ocean-sdk uses 3.13 + - image: cimg/python:3.13 # As of April 2026, the dwave-ocean-sdk uses 3.13 steps: - checkout @@ -337,7 +337,7 @@ jobs: deploy: docker: - - image: python:3.10 + - image: cimg/python:3.10 steps: - attach_workspace: From ef950e42b4c04022c4a1061e815b6e2aba30a1fd Mon Sep 17 00:00:00 2001 From: Alexander Condello Date: Thu, 23 Jul 2026 15:28:45 -0700 Subject: [PATCH 9/9] Add NumberNode::checkpoint() --- .../dwave-optimization/nodes/numbers.hpp | 7 +- dwave/optimization/src/nodes/_checkpoints.cpp | 3 + dwave/optimization/src/nodes/_checkpoints.hpp | 3 + dwave/optimization/src/nodes/_state.hpp | 23 +++ dwave/optimization/src/nodes/numbers.cpp | 136 +++++++++++++++- tests/cpp/nodes/test_collections.cpp | 2 + tests/cpp/nodes/test_numbers.cpp | 149 +++++++++++++++++- 7 files changed, 313 insertions(+), 10 deletions(-) diff --git a/dwave/optimization/include/dwave-optimization/nodes/numbers.hpp b/dwave/optimization/include/dwave-optimization/nodes/numbers.hpp index 252e709f..34cc9069 100644 --- a/dwave/optimization/include/dwave-optimization/nodes/numbers.hpp +++ b/dwave/optimization/include/dwave-optimization/nodes/numbers.hpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include "dwave-optimization/array.hpp" @@ -122,6 +121,8 @@ class NumberNode : public ArrayOutputMixin, public DecisionNode { // NumberNode methods ***************************************************** + std::unique_ptr checkpoint(State& state) const; + // In the given state, swap the value of index i with the value of index j. // Users may pass the slices (per sum constraint) that each index lies on. void exchange( @@ -290,6 +291,10 @@ class IntegerNode : public NumberNode { // IntegerNode methods **************************************************** + /// Set the current state to match the one at the time the given checkpoint was created. + void assign_from_checkpoint(State& state, checkpoint_type& checkpoint) const; + void assign_from_checkpoint(State& state, checkpoint_type&& checkpoint) const; + // Set the value at the given index in the given state. // Users may pass the slices (per sum constraint) that each index lies on. void set_value( diff --git a/dwave/optimization/src/nodes/_checkpoints.cpp b/dwave/optimization/src/nodes/_checkpoints.cpp index 60a4d8bc..4b3fe59b 100644 --- a/dwave/optimization/src/nodes/_checkpoints.cpp +++ b/dwave/optimization/src/nodes/_checkpoints.cpp @@ -41,6 +41,9 @@ LinkedListCheckpoint::~LinkedListCheckpoint() { ); } +DiffCheckpoint::DiffCheckpoint(CheckpointableState& state, ssize_t drop) : + LinkedListCheckpoint(state), updates_(), drop_(drop) {} + DiffCheckpoint::DiffCheckpoint(CheckpointableState& state, std::span diff) : LinkedListCheckpoint(state), updates_(), drop_(diff.size()) { if (auto* prev_ptr = static_cast(prev_ptr_)) { diff --git a/dwave/optimization/src/nodes/_checkpoints.hpp b/dwave/optimization/src/nodes/_checkpoints.hpp index 39ac72db..8e1866d7 100644 --- a/dwave/optimization/src/nodes/_checkpoints.hpp +++ b/dwave/optimization/src/nodes/_checkpoints.hpp @@ -87,6 +87,9 @@ class DiffCheckpoint : public LinkedListCheckpoint { void revert_updates(std::vector updates); + protected: + DiffCheckpoint(CheckpointableState& state, ssize_t drop); + private: std::vector> updates_; ssize_t drop_; diff --git a/dwave/optimization/src/nodes/_state.hpp b/dwave/optimization/src/nodes/_state.hpp index 5f4c8162..b5e4d812 100644 --- a/dwave/optimization/src/nodes/_state.hpp +++ b/dwave/optimization/src/nodes/_state.hpp @@ -165,6 +165,29 @@ class ArrayStateData { size_ = buffer.size(); } + // Commit the changes and clear the diff by returning the diff buffer. + std::vector revert_and_detach() { + assert(previous_size_ >= 0); + buffer.resize(previous_size_); + const ssize_t size = buffer.size(); + for (const auto& [index, old, _] : updates | std::views::reverse) { + assert(index >= 0); + if (index >= size) continue; + buffer[index] = old; + } + size_ = buffer.size(); + + std::vector tmp; + std::swap(updates, tmp); + // AlexC: we could now do updates.reserve(tmp.size()) under the assumption + // that future update buffers will be a similar size. On the other hand, + // not doing this provides another meaningful difference to ::commit(). + // For now, I think it make sense to not but performance testing needed. + + assert(updates.empty()); + return tmp; + } + // Set the value at index, tracking the change in the diff. // If allow_emplace is true, do an emplace_back iff the index is equal to the current size. bool set(ssize_t i, double value, bool allow_emplace = false) { diff --git a/dwave/optimization/src/nodes/numbers.cpp b/dwave/optimization/src/nodes/numbers.cpp index 317082ed..30920d53 100644 --- a/dwave/optimization/src/nodes/numbers.cpp +++ b/dwave/optimization/src/nodes/numbers.cpp @@ -23,6 +23,7 @@ #include #include +#include "_checkpoints.hpp" #include "_state.hpp" #include "dwave-optimization/array.hpp" #include "dwave-optimization/common.hpp" @@ -72,8 +73,75 @@ NumberNode::SumConstraint::Operator NumberNode::SumConstraint::op(const ssize_t return operators_[slice]; } +class NumberNodeCheckpoint_ : public DiffCheckpoint { + public: + using slice_cache_type = std::vector>; + + NumberNodeCheckpoint_( + CheckpointableState& state, + std::span diff, + const slice_cache_type& slice_cache + ) : + DiffCheckpoint(state, diff.size()), slice_caches_() { + // If there is an older checkpoint, we want to put anything we're currently + // holding in our slice cache onto it + if (auto* prev_ptr = static_cast(prev_ptr_)) { + prev_ptr->commit_updates(std::vector(diff.begin(), diff.end()), slice_cache); + assert(prev_ptr->drop() == 0); + } + } + + void commit_updates(std::vector updates, slice_cache_type slice_cache) { + ssize_t drop = this->drop(); + assert(0 <= drop and static_cast(drop) <= updates.size()); + + if (not slice_cache.empty()) { + assert(updates.size() == slice_cache.size()); + + if (drop) { + slice_cache.erase(slice_cache.begin(), slice_cache.begin() + drop); + } + slice_caches_.emplace_back(std::move(slice_cache)); + } + + DiffCheckpoint::commit_updates(std::move(updates)); + assert(this->drop() == 0); + } + + auto detach_slice_cache() { + using join_type = decltype(std::move(slice_caches_) | std::views::join); + + if (slice_caches_.empty()) return std::optional(); + + auto joined = std::move(slice_caches_) | std::views::join; + assert(slice_caches_.empty()); + return std::optional(std::move(joined)); + } + + void revert_updates(std::vector updates, slice_cache_type slice_cache) { + ssize_t drop = this->drop(); + assert(0 <= drop and static_cast(drop) <= updates.size()); + + if (not slice_cache.empty()) { + assert(updates.size() == slice_cache.size()); + + if (drop) { + slice_cache.erase(slice_cache.begin() + drop, slice_cache.end()); + } + std::reverse(slice_cache.begin(), slice_cache.end()); + slice_caches_.emplace_back(std::move(slice_cache)); + } + + DiffCheckpoint::revert_updates(std::move(updates)); + assert(this->drop() == 0); + } + + private: + std::vector slice_caches_; +}; + /// State dependent data attached to NumberNode -struct NumberNodeStateData : public ArrayNodeStateData { +class NumberNodeStateData : public ArrayNodeStateData, public CheckpointableState { public: // User does not provide sum constraints. NumberNodeStateData(std::vector input) : ArrayNodeStateData(std::move(input)) {} @@ -84,14 +152,28 @@ struct NumberNodeStateData : public ArrayNodeStateData { ) : ArrayNodeStateData(std::move(input)), sum_constraints_lhs(std::move(sum_constraints_lhs)) {} + std::unique_ptr checkpoint() { + return std::make_unique(*this, this->diff(), this->slice_cache_); + } + std::unique_ptr copy() const override { return std::make_unique(*this); } /// Commit the state dependent data of NumberNode. void commit() { - ArrayNodeStateData::commit(); // Commit changes to the buffer. - slice_cache_.clear(); // Empty the slice cache. + if (auto* checkpoint_ptr = this->checkpoint_ptr()) { + checkpoint_ptr->commit_updates( + ArrayNodeStateData::commit_and_detach(), std::move(slice_cache_) + ); + } else { + ArrayNodeStateData::commit(); // Commit changes to the buffer. + slice_cache_.clear(); // Empty the slice cache. + } + + // everything should have been cleared out regardless of which path we took + assert(this->diff().empty()); + assert(slice_cache_.empty()); } /// Revert the state dependent data of NumberNode. @@ -142,10 +224,16 @@ void NumberNodeStateData::revert() { sum_constraints_lhs[j][slices[j]] -= difference; } } - slice_cache_.clear(); // Empty the slice cache. } - ArrayNodeStateData::revert(); // Revert changes to the buffer. + if (auto* checkpoint_ptr = this->checkpoint_ptr()) { + checkpoint_ptr->revert_updates( + ArrayNodeStateData::revert_and_detach(), std::move(slice_cache_) + ); + } else { + slice_cache_.clear(); // Empty the slice cache. + ArrayNodeStateData::revert(); // Revert changes to the buffer. + } } void NumberNodeStateData::update( @@ -529,6 +617,10 @@ void NumberNode::propagate(State& state) const { } } +std::unique_ptr NumberNode::checkpoint(State& state) const { + return data_ptr_(state)->checkpoint(); +} + void NumberNode::commit(State& state) const noexcept { data_ptr_(state)->commit(); } @@ -953,6 +1045,40 @@ IntegerNode::IntegerNode( std::move(sum_constraints) ) {} +void IntegerNode::assign_from_checkpoint(State& state, checkpoint_type& checkpoint) const { + auto state_data = data_ptr_(state); + + auto* checkpoint_ptr = static_cast(checkpoint.get()); + + // todo: assert that this checkpoint is the latest + + auto updates = checkpoint_ptr->detach_updates(); + auto slice_cache = checkpoint_ptr->detach_slice_cache(); // this is an std::optional<...>! + + if (slice_cache.has_value()) { + assert(sum_constraints_.size() > 0); + + auto slices_rit = std::ranges::rbegin(*slice_cache); + + for (const auto& [idx, old, _] : std::move(updates) | std::views::reverse) { + state_data->set(idx, old); + state_data->update(*this, idx, old - diff(state).back().old, *(slices_rit++)); + } + } else { + assert(updates.empty() or sum_constraints_.empty()); + + // in this case we don't need to do anything to update the slice data + for (const auto& [idx, old, _] : std::move(updates) | std::views::reverse) { + state_data->set(idx, old); + } + } +} + +void IntegerNode::assign_from_checkpoint(State& state, checkpoint_type&& checkpoint) const { + assign_from_checkpoint(state, checkpoint); // call the lvalue version + checkpoint.reset(); +} + bool IntegerNode::integral() const { return true; } bool IntegerNode::is_valid(ssize_t index, double value) const { diff --git a/tests/cpp/nodes/test_collections.cpp b/tests/cpp/nodes/test_collections.cpp index c8ac10d0..b7820e5f 100644 --- a/tests/cpp/nodes/test_collections.cpp +++ b/tests/cpp/nodes/test_collections.cpp @@ -873,6 +873,8 @@ TEST_CASE("SetNode") { graph.propose(state); THEN("we can go backwards through them without commiting and everything is correct") { + // TODO: check mutating before assigning + set_ptr->assign_from_checkpoint(state, std::move(checkpoint4)); CHECK_THAT(set_ptr->view(state), RangeEquals({2})); diff --git a/tests/cpp/nodes/test_numbers.cpp b/tests/cpp/nodes/test_numbers.cpp index 4349961d..0e10ae0d 100644 --- a/tests/cpp/nodes/test_numbers.cpp +++ b/tests/cpp/nodes/test_numbers.cpp @@ -23,6 +23,7 @@ #include "dwave-optimization/graph.hpp" #include "dwave-optimization/nodes/numbers.hpp" +#include "dwave-optimization/nodes/testing.hpp" using Catch::Matchers::RangeEquals; @@ -1892,6 +1893,8 @@ TEST_CASE("IntegerNode") { GIVEN("An Integer Node representing an 1d array of 10 elements with lower bound -10") { auto ptr = graph.emplace_node(std::initializer_list{10}, -10); + graph.emplace_node(ptr); + THEN("The shape is fixed") { CHECK(ptr->ndim() == 1); CHECK(ptr->size() == 10); @@ -1985,6 +1988,46 @@ TEST_CASE("IntegerNode") { } } } + + AND_WHEN("We checkpoint the state and then mutate") { + auto checkpoint = ptr->checkpoint(state); // [-4, -4, -2, -2, 0, 0, 2, 2, 4, 4] + + ptr->exchange(state, 0, 2); // [-2, -4, -4, -2, 0, 0, 2, 2, 4, 4] + ptr->set_value(state, 3, 1); // [-2, -4, -4, 1, 0, 0, 2, 2, 4, 4] + + THEN("We can commit, then assign from the checkpoint") { + graph.propose(state); + + ptr->assign_from_checkpoint(state, checkpoint); + + CHECK_THAT(ptr->view(state), RangeEquals({-4, -4, -2, -2, 0, 0, 2, 2, 4, 4})); + } + } + + AND_WHEN("We mutate, checkpoint the state, and then mutate again") { + ptr->set_value(state, 3, 1); // [-4, -4, -2, 1, 0, 0, 2, 2, 4, 4] + + auto checkpoint = ptr->checkpoint(state); + + ptr->exchange(state, 0, 2); // [-2, -4, -4, 1, 0, 0, 2, 2, 4, 4] + + THEN("We can commit, then assign from the checkpoint") { + graph.propose(state); + + ptr->assign_from_checkpoint(state, checkpoint); + + CHECK_THAT(ptr->view(state), RangeEquals({-4, -4, -2, 1, 0, 0, 2, 2, 4, 4})); + } + + THEN("We can revert, then assign from the checkpoint") { + graph.propagate(state); + graph.revert(state); + + ptr->assign_from_checkpoint(state, checkpoint); + + CHECK_THAT(ptr->view(state), RangeEquals({-4, -4, -2, 1, 0, 0, 2, 2, 4, 4})); + } + } } } @@ -2195,6 +2238,8 @@ TEST_CASE("IntegerNode") { std::initializer_list{2, 2, 2}, -5, 8, sum_constraints ); + graph.emplace_node(inode_ptr); + THEN("Sum constraint is correct") { CHECK(inode_ptr->sum_constraints().size() == 1); SumConstraint inode_sum_constraint = inode_ptr->sum_constraints()[0]; @@ -2209,14 +2254,110 @@ TEST_CASE("IntegerNode") { auto state = graph.initialize_state(); graph.initialize_state(state); std::vector expected_init{8, 8, 8, 8, 8, 8, -3, -5}; - auto sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); THEN("Sum constraint sums and state are correct") { - CHECK(inode_ptr->sum_constraints_lhs(state).size() == 1); - CHECK(inode_ptr->sum_constraints_lhs(state).data()[0].size() == 1); - CHECK_THAT(inode_ptr->sum_constraints_lhs(state)[0], RangeEquals({40})); + auto sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({40})); CHECK_THAT(inode_ptr->view(state), RangeEquals(expected_init)); } + + AND_WHEN("We create a checkpoint and then mutate the state") { + auto checkpoint = inode_ptr->checkpoint(state); + + inode_ptr->set_value(state, 7, 3); // [ 8, 8, 8, 8, 8, 8, -3, 3 ] + inode_ptr->exchange(state, 1, 6); // [ 8, -3, 8, 8, 8, 8, 8, 3 ] + + THEN("After committing, We can revert to that checkpoint") { + graph.propose(state); + inode_ptr->assign_from_checkpoint(state, std::move(checkpoint)); + + auto sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({40})); + CHECK_THAT(inode_ptr->view(state), RangeEquals(expected_init)); + } + } + + AND_WHEN("We mutate, create a checkpoint, and then mutate some more") { + inode_ptr->set_value(state, 7, 3); // [ 8, 8, 8, 8, 8, 8, -3, 3 ] + auto checkpoint = inode_ptr->checkpoint(state); + inode_ptr->exchange(state, 1, 6); // [ 8, -3, 8, 8, 8, 8, 8, 3 ] + + THEN("After committing, we can assign from that checkpoint") { + graph.propose(state); + inode_ptr->assign_from_checkpoint(state, std::move(checkpoint)); + + auto sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({48})); + CHECK_THAT(inode_ptr->view(state), RangeEquals({8, 8, 8, 8, 8, 8, -3, 3})); + } + + THEN("After reverting, we can assign from that checkpoint") { + graph.propagate(state); + graph.revert(state); + inode_ptr->assign_from_checkpoint(state, std::move(checkpoint)); + + auto sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({48})); + CHECK_THAT(inode_ptr->view(state), RangeEquals({8, 8, 8, 8, 8, 8, -3, 3})); + } + + AND_WHEN("We create a new checkpoint") { + auto checkpoint1 = inode_ptr->checkpoint(state); + + THEN("We can commit, and restore the checkpoints") { + inode_ptr->exchange(state, 1, 2); // [ 8, 8, -3, 8, 8, 8, 8, 3 ] + graph.propose(state); + + inode_ptr->assign_from_checkpoint(state, std::move(checkpoint1)); + + auto sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({48})); + CHECK_THAT(inode_ptr->view(state), RangeEquals({8, -3, 8, 8, 8, 8, 8, 3})); + + inode_ptr->assign_from_checkpoint(state, std::move(checkpoint)); + + sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({48})); + CHECK_THAT(inode_ptr->view(state), RangeEquals({8, 8, 8, 8, 8, 8, -3, 3})); + } + + THEN("We can revert, and restore the checkpoints") { + inode_ptr->exchange(state, 1, 2); // [ 8, 8, -3, 8, 8, 8, 8, 3 ] + graph.propagate(state); + graph.revert(state); + + inode_ptr->assign_from_checkpoint(state, std::move(checkpoint1)); + + auto sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({48})); + CHECK_THAT(inode_ptr->view(state), RangeEquals({8, -3, 8, 8, 8, 8, 8, 3})); + + inode_ptr->assign_from_checkpoint(state, std::move(checkpoint)); + + sum_constraints_lhs = inode_ptr->sum_constraints_lhs(state); + CHECK(sum_constraints_lhs.size() == 1); + CHECK(sum_constraints_lhs.data()[0].size() == 1); + CHECK_THAT(sum_constraints_lhs[0], RangeEquals({48})); + CHECK_THAT(inode_ptr->view(state), RangeEquals({8, 8, 8, 8, 8, 8, -3, 3})); + } + } + } } }