Implement state checkpointing for decision nodes by arcondello · Pull Request #587 · dwavesystems/dwave-optimization

arcondello · 2026-07-17T23:22:43Z

Closes #510
Closes #552 by replacing it

AI Generation Disclosure

No AI was used to write the code. I intend to use it to review this PR.

wbernoudy · 2026-07-20T16:54:39Z


+CollectionCheckpoint_::CollectionCheckpoint_(CollectionStateData_* state_ptr) :
+    updates_(),
+    drop_(state_ptr->all_updates_.size()),  // so we ignore any updates added before we're made


What happens if some mutations are made to the node, then you make a checkpoint (causing the drop_ to be set > 0), and then you revert, make more mutations, and commit? Won't this cause some updates to be "dropped" that are actually relevant to the checkpoint?

Actually, I think I mean "make another checkpoint" as final step in the example rather than commit.

Yup! Good catch. Fixed in 214c5e2

This avoids using GCC11 which had some bugs in their ranges implementation.

arcondello · 2026-07-27T18:47:15Z


+    // Overloads required by the DecisionNode ABC *****************************
+
+    // DynamicArrayTestingNode does not impement checkpointing


See #593

Also I see the typo here.

arcondello · 2026-07-27T20:13:26Z

    }
 }

+void Graph::propose(State& state) const {


Could be smarter with #589 or #592, but that's for another PR.

arcondello · 2026-07-27T20:13:49Z

+
+                set_ptr->assign_from_checkpoint(state, std::move(checkpoint0));
+
+                // CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1}));


oops, will restore

arcondello · 2026-07-27T20:14:02Z

+            }
+        }
+
+        // TODO: within one propagation


this is done, need to remove this comment

fastbodin · 2026-07-28T19:37:57Z

+                auto checkpoint = ptr->checkpoint(state);  // 0, 1, 0, 1, 0, 1, 0, 1, 0, 1
+
+                THEN("We can mutate and then assign from that checkpoint") {
+                    ptr->set_value(state, 0, 1);  // 0, 1, 0, 1, 0, 1, 0, 1, 0, 1


// 1, 1, 0, 1, 0, 1, 0, 1, 0, 1

since you called set_value()

fastbodin · 2026-07-28T19:41:06Z

+                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") {


Nit:

THEN("We can propose, then assign from the checkpoint") {

I realize propose is propagate and commit. As this is a Nit, I am not going to comment at all duplicate instances.

fastbodin · 2026-07-28T19:41:45Z

+            AND_WHEN("We create a checkpoint to that state") {
+                auto checkpoint = ptr->checkpoint(state);  // 0, 1, 0, 1, 0, 1, 0, 1, 0, 1
+
+                THEN("We can mutate and then assign from that checkpoint") {


Nit

THEN("We can mutate, then propose, and then assign from that checkpoint") {

fastbodin · 2026-07-28T23:11:00Z

+                        CHECK_THAT(inode_ptr->view(state), RangeEquals({8, 8, 8, 8, 8, 8, -3, 3}));
+                    }
+                }
+            }


A bit of a pain, but I think it might be worth adding a single test for BinaryNode with checkpointing to check that all the correct indices are cached with 1s and 0s (I am referring to the BinaryNode DisjointSparseSet).

THEN("Sum constraint sums, tracked indices, and state are correct") { CHECK(bnode_ptr->sum_constraints_lhs(state).size() == 1); CHECK(bnode_ptr->sum_constraints_lhs(state).data()[0].size() == 1); CHECK_THAT(bnode_ptr->sum_constraints_lhs(state)[0], RangeEquals({3})); check_indices<true>(state, bnode_ptr, 0, 0, {1, 2, 5}); // <- ***** This check is the important one ****** check_indices<false>(state, bnode_ptr, 0, 0, {0, 3, 4}); // <- ***** And this one ****** CHECK_THAT(bnode_ptr->view(state), RangeEquals(expected_init)); }

fastbodin · 2026-07-28T23:17:31Z


+class NumberNodeCheckpoint_ : public DiffCheckpoint {
+ public:
+    using slice_cache_type = std::vector<std::vector<ssize_t>>;


Wishing I had used this elsewhere 😂

fastbodin · 2026-07-28T23:40:48Z

        assert(size_ >= 0 && static_cast<std::size_t>(size_) == buffer.size());
    }

+    // Commit the changes and clear the diff by returning the diff buffer.


Confused where the commit() is and am feeling like I am missing something.

fastbodin · 2026-07-28T23:42:26Z

        size_ = buffer.size();
    }

+    // Commit the changes and clear the diff by returning the diff buffer.


Same doc string comments as commit_and_detach(). Intentional?

fastbodin · 2026-07-28T23:50:13Z

+        return static_cast<T*>(prev_ptr_);
+    }
+
+ private:  // todo: private?


Hanging comment?

arcondello added the enhancement New feature or request label Jul 17, 2026

Implement state checkpointing for CollectionNode

953304e

arcondello force-pushed the feature/decision-checkpoints branch from 6df7006 to 953304e Compare July 17, 2026 23:25

wbernoudy reviewed Jul 20, 2026

View reviewed changes

Fix reverts and dangling pointers for CollectionNode checkpoints

214c5e2

arcondello force-pushed the feature/decision-checkpoints branch 2 times, most recently from b4679c2 to 214c5e2 Compare July 20, 2026 18:59

Use offical Python images in CircleCI

9cd035b

This avoids using GCC11 which had some bugs in their ranges implementation.

arcondello force-pushed the feature/decision-checkpoints branch from 86b97e8 to 9cd035b Compare July 20, 2026 19:18

arcondello commented Jul 20, 2026

View reviewed changes

Comment thread dwave/optimization/include/dwave-optimization/state.hpp Outdated

arcondello added 4 commits July 20, 2026 15:05

Add LinkedListCheckpoint helper class

6ec49f9

Fix the copying of checkpointed states

fc44824

Drop unused NodeStateCheckpoint::valid() method

f711e63

Add DiffCheckpoint class for checkpoints that track diffs

e85f85f

arcondello force-pushed the feature/decision-checkpoints branch 2 times, most recently from f1aa563 to 06b9dc5 Compare July 21, 2026 21:20

Go back to using CircleCI image for some CI tasks

f70699e

arcondello force-pushed the feature/decision-checkpoints branch from 06b9dc5 to f70699e Compare July 21, 2026 21:21

arcondello added 3 commits July 23, 2026 15:28

Add NumberNode::checkpoint()

ef950e4

Fix assigning from a checkpoint after mutation

e61a70f

Add test for BinaryNode::assign_from_checkpoint

e845381

arcondello force-pushed the feature/decision-checkpoints branch from 30637e6 to 5fbe15b Compare July 27, 2026 18:46

arcondello changed the title ~~Implement state checkpointing for CollectionNode~~ Implement state checkpointing for decision nodes Jul 27, 2026

arcondello commented Jul 27, 2026

View reviewed changes

Make all DecisionNodes implement checkpointing

4402568

arcondello force-pushed the feature/decision-checkpoints branch from 5fbe15b to 4402568 Compare July 27, 2026 18:51

arcondello marked this pull request as ready for review July 27, 2026 18:51

arcondello requested a review from fastbodin July 27, 2026 18:52

arcondello commented Jul 27, 2026

View reviewed changes

fastbodin reviewed Jul 29, 2026

View reviewed changes


		// Overloads required by the DecisionNode ABC *****************************

		// DynamicArrayTestingNode does not impement checkpointing


		set_ptr->assign_from_checkpoint(state, std::move(checkpoint0));

		// CHECK_THAT(set_ptr->view(state), RangeEquals({0, 1}));

Uh oh!

Conversation

arcondello commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI Generation Disclosure

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arcondello Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arcondello Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arcondello commented Jul 17, 2026 •

edited

Loading

arcondello Jul 20, 2026 •

edited

Loading

arcondello Jul 27, 2026 •

edited

Loading