From 95b80ba27f35cc5bc867e93ac955b8456a1c0754 Mon Sep 17 00:00:00 2001 From: William Bernoudy Date: Wed, 22 Jul 2026 09:47:35 -0700 Subject: [PATCH 1/3] Make `State` a class and add `Graph::mutated()` --- .../include/dwave-optimization/graph.hpp | 23 ++++++++++- .../include/dwave-optimization/state.hpp | 39 ++++++++++++++++++- dwave/optimization/src/graph.cpp | 31 ++++++++++++++- tests/cpp/test_graph.cpp | 18 ++++++++- 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/dwave/optimization/include/dwave-optimization/graph.hpp b/dwave/optimization/include/dwave-optimization/graph.hpp index 8009de45..105aeb55 100644 --- a/dwave/optimization/include/dwave-optimization/graph.hpp +++ b/dwave/optimization/include/dwave-optimization/graph.hpp @@ -40,6 +40,8 @@ class Node; struct Decision {}; class Graph { + friend class State; + public: Graph() noexcept = default; ~Graph() noexcept = default; @@ -115,6 +117,25 @@ class Graph { std::span inputs() noexcept { return inputs_; } std::span inputs() const noexcept { return inputs_; } + /// Return the decision nodes that have been "mutated", meaning that they + /// have pending changes that must be propagated before committing or + /// reverting. Equivalently, the combined descendants of the returned nodes + /// are guaranteed to be a superset of the nodes that require having + /// `propagate()` and then `commit()` or `revert()` called on them. + /// + /// Notes: + /// - Using this method in conjunction with `descendants()` and + /// `propagate()`/`commit()`/`revert()` may be inefficient compared to + /// doing these manually in the case where you know only some descendants + /// of one or more of the decision nodes are relevant, e.g. only one of the + /// `DisjointListNode` successors of `DisjointListsNode` has pending + /// changes and the rest of the `DisjointListNode`s (and their descendants) + /// can be ignored + /// - This method will return the same nodes before and after calling + /// `propagate()`. Only after committing/reverting will the returned list + /// be empty again. + std::span mutated(State& state) const; + /// All of the nodes in the graph. std::span> nodes() const { return nodes_; } @@ -143,7 +164,7 @@ class Graph { void propagate(State& state) const; /// Call the propagate method on each node in changed. Note this does not call propagate on - /// the descendents of changed. + /// the descendants of changed. void propagate(State& state, std::span changed) const; void propagate(State& state, std::vector&& changed) const; diff --git a/dwave/optimization/include/dwave-optimization/state.hpp b/dwave/optimization/include/dwave-optimization/state.hpp index 2228ddbb..9232dc0b 100644 --- a/dwave/optimization/include/dwave-optimization/state.hpp +++ b/dwave/optimization/include/dwave-optimization/state.hpp @@ -14,9 +14,9 @@ #pragma once +#include #include #include -#include namespace dwave::optimization { @@ -34,7 +34,42 @@ struct NodeStateData { bool mark = false; }; -using State = typename std::vector>; +// Foward declaration +class DecisionNode; + +class State { + friend class Graph; + + public: + template + State(Args&&... args) : node_data_(std::forward(args)...) {} + + template + void emplace_back(Args&&... args) { + node_data_.emplace_back(std::forward(args)...); + } + + template + auto& operator[](index_type index) { + return node_data_[index]; + } + + template + auto& operator[](index_type index) const { + return node_data_[index]; + } + + template + void resize(size_type size) { + node_data_.resize(size); + } + + size_t size() const { return node_data_.size(); } + + private: + std::vector> node_data_; + std::vector mutated_nodes_; +}; /// A generic base class for node checkpoints. struct NodeStateCheckpoint { diff --git a/dwave/optimization/src/graph.cpp b/dwave/optimization/src/graph.cpp index 50473f9a..1bd22bdb 100644 --- a/dwave/optimization/src/graph.cpp +++ b/dwave/optimization/src/graph.cpp @@ -28,6 +28,7 @@ #endif #include "dwave-optimization/array.hpp" +#include "dwave-optimization/nodes/collections.hpp" #include "dwave-optimization/nodes/constants.hpp" #include "dwave-optimization/nodes/inputs.hpp" @@ -157,6 +158,34 @@ void Graph::initialize_state(State& state) { static_cast(this)->initialize_state(state); } +std::span Graph::mutated(State& state) const { + state.mutated_nodes_.clear(); + for (const DecisionNode* dec_ptr : decisions()) { + if (const ArrayNode* arr_ptr = dynamic_cast(dec_ptr); arr_ptr) { + if (not arr_ptr->diff(state).empty()) { + state.mutated_nodes_.push_back(dec_ptr); + } + } else if ( + dynamic_cast(dec_ptr) or + dynamic_cast(dec_ptr) + ) { + for (const Node* suc_ptr : dec_ptr->successors()) { + const ArrayNode* arr_ptr = dynamic_cast(suc_ptr); + assert(arr_ptr and "all successors should be array nodes"); + if (not arr_ptr->diff(state).empty()) { + state.mutated_nodes_.push_back(dec_ptr); + break; + } + } + } else { + assert(false and "unknown decision node type"); + unreachable(); + } + } + + return state.mutated_nodes_; +} + void Graph::pop_decision() { assert(not topologically_sorted_ and "cannot pop a decision from a locked model"); assert(not decisions_.empty() and "need at least one decision"); @@ -493,7 +522,7 @@ ssize_t Graph::remove_unused_nodes(bool ignore_listeners) { for (auto& uptr : nodes_ | std::views::reverse) { if (uptr->topological_index_ == keep) continue; // we marked these to keep - if (uptr->successors().size() > 0) continue; // this node is used by other nodes + if (uptr->successors().size() > 0) continue; // this node is used by other nodes // We have a node with no successors and that we haven't marked it as important. // So let's mark it to be dropped later. diff --git a/tests/cpp/test_graph.cpp b/tests/cpp/test_graph.cpp index 60d15dc8..5e1ae816 100644 --- a/tests/cpp/test_graph.cpp +++ b/tests/cpp/test_graph.cpp @@ -235,7 +235,9 @@ TEST_CASE("Graph constructors, assignment operators, and swapping") { } } -TEST_CASE("Graph::commit(), Graph::descendants(), Graph::propagate(), and Graph::revert") { +TEST_CASE( + "Graph::commit(), Graph::descendants(), Graph::mutated(), Graph::propagate(), and Graph::revert" +) { auto graph = Graph(); auto* x_ptr = graph.emplace_node(); auto* y_ptr = graph.emplace_node(); @@ -248,17 +250,23 @@ TEST_CASE("Graph::commit(), Graph::descendants(), Graph::propagate(), and Graph: CHECK_THAT(descendants, RangeEquals(std::vector{x_ptr, z_ptr})); } SECTION("Propagate all") { + CHECK_THAT(graph.mutated(state), Catch::Matchers::RangeEquals(std::vector{})); + CHECK(x_ptr->view(state).front() == 0); CHECK(y_ptr->view(state).front() == 0); CHECK(z_ptr->view(state).front() == 0); x_ptr->flip(state, 0); + CHECK_THAT(graph.mutated(state), Catch::Matchers::RangeEquals({x_ptr})); + y_ptr->flip(state, 0); CHECK(x_ptr->diff(state).size()); CHECK(y_ptr->diff(state).size()); CHECK(z_ptr->diff(state).empty()); // not yet propagated to + CHECK_THAT(graph.mutated(state), Catch::Matchers::RangeEquals({x_ptr, y_ptr})); + graph.propagate(state); CHECK(x_ptr->view(state).front() == 1); @@ -269,6 +277,8 @@ TEST_CASE("Graph::commit(), Graph::descendants(), Graph::propagate(), and Graph: CHECK(y_ptr->diff(state).size()); CHECK(z_ptr->diff(state).size()); // now has pending changes + CHECK_THAT(graph.mutated(state), Catch::Matchers::RangeEquals({x_ptr, y_ptr})); + SECTION("Commit all") { graph.commit(state); @@ -280,6 +290,9 @@ TEST_CASE("Graph::commit(), Graph::descendants(), Graph::propagate(), and Graph: CHECK(x_ptr->diff(state).empty()); CHECK(y_ptr->diff(state).empty()); CHECK(z_ptr->diff(state).empty()); + + // Committing should reset the mutated nodes + CHECK_THAT(graph.mutated(state), Catch::Matchers::RangeEquals(std::vector{})); } SECTION("Revert all") { @@ -293,6 +306,9 @@ TEST_CASE("Graph::commit(), Graph::descendants(), Graph::propagate(), and Graph: CHECK(x_ptr->diff(state).empty()); CHECK(y_ptr->diff(state).empty()); CHECK(z_ptr->diff(state).empty()); + + // Reverting should reset the mutated nodes + CHECK_THAT(graph.mutated(state), Catch::Matchers::RangeEquals(std::vector{})); } } } From 2041706ac1d458fab6e8ea12a9661fe1fa4da266 Mon Sep 17 00:00:00 2001 From: William Bernoudy Date: Fri, 31 Jul 2026 12:19:15 -0700 Subject: [PATCH 2/3] Simplify new State interface further --- .../include/dwave-optimization/graph.hpp | 6 +++--- .../include/dwave-optimization/state.hpp | 17 ++++------------- dwave/optimization/src/graph.cpp | 11 ++++++++--- dwave/optimization/src/nodes/lambda.cpp | 3 +-- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/dwave/optimization/include/dwave-optimization/graph.hpp b/dwave/optimization/include/dwave-optimization/graph.hpp index 105aeb55..00ec336e 100644 --- a/dwave/optimization/include/dwave-optimization/graph.hpp +++ b/dwave/optimization/include/dwave-optimization/graph.hpp @@ -406,7 +406,7 @@ class Node { StateData* data_ptr_(State& state) const { const ssize_t index = topological_index(); assert(index >= 0 and "must be topologically sorted"); - assert(state.size() > static_cast(index) and "unexpected state length"); + assert(state.size() > index and "unexpected state length"); assert(state[index] != nullptr and "uninitialized state"); return static_cast(state[index].get()); @@ -415,7 +415,7 @@ class Node { const StateData* data_ptr_(const State& state) const { const ssize_t index = topological_index(); assert(index >= 0 and "must be topologically sorted"); - assert(state.size() > static_cast(index) and "unexpected state length"); + assert(state.size() > index and "unexpected state length"); assert(state[index] != nullptr and "uninitialized state"); return static_cast(state[index].get()); @@ -425,7 +425,7 @@ class Node { void emplace_data_ptr_(State& state, Args&&... args) const { const ssize_t index = topological_index(); assert(index >= 0 and "must be topologically sorted"); - assert(state.size() > static_cast(index) and "unexpected state length"); + assert(state.size() > index and "unexpected state length"); assert(state[index] == nullptr and "already initialized state"); state[index] = std::make_unique(std::forward(args)...); diff --git a/dwave/optimization/include/dwave-optimization/state.hpp b/dwave/optimization/include/dwave-optimization/state.hpp index 9232dc0b..5e61239b 100644 --- a/dwave/optimization/include/dwave-optimization/state.hpp +++ b/dwave/optimization/include/dwave-optimization/state.hpp @@ -34,20 +34,14 @@ struct NodeStateData { bool mark = false; }; -// Foward declaration +// Foward declaration for storing the mutated decision nodes on State class DecisionNode; class State { friend class Graph; public: - template - State(Args&&... args) : node_data_(std::forward(args)...) {} - - template - void emplace_back(Args&&... args) { - node_data_.emplace_back(std::forward(args)...); - } + State(ssize_t size = 0) : node_data_(size) {} template auto& operator[](index_type index) { @@ -59,12 +53,9 @@ class State { return node_data_[index]; } - template - void resize(size_type size) { - node_data_.resize(size); - } + void resize(ssize_t size) { node_data_.resize(size); } - size_t size() const { return node_data_.size(); } + ssize_t size() const { return node_data_.size(); } private: std::vector> node_data_; diff --git a/dwave/optimization/src/graph.cpp b/dwave/optimization/src/graph.cpp index 1bd22bdb..7d60e330 100644 --- a/dwave/optimization/src/graph.cpp +++ b/dwave/optimization/src/graph.cpp @@ -105,9 +105,9 @@ std::vector Graph::descendants(State& state, std::vector Graph::descendants(std::vector sources) const { - State state; + State state(num_nodes()); for (ssize_t i = 0, stop = num_nodes(); i < stop; ++i) { - state.emplace_back(std::make_unique()); + state[i] = std::make_unique(); } return descendants(state, sources); } @@ -159,9 +159,14 @@ void Graph::initialize_state(State& state) { } std::span Graph::mutated(State& state) const { + // We will want to eventually replace this implementation with an approach where + // decision nodes "eagerly" add themselves to the list of mutated nodes after they + // are mutated. This will avoid the need to iterate over all decision nodes every + // time this method is called. state.mutated_nodes_.clear(); + for (const DecisionNode* dec_ptr : decisions()) { - if (const ArrayNode* arr_ptr = dynamic_cast(dec_ptr); arr_ptr) { + if (auto* arr_ptr = dynamic_cast(dec_ptr); arr_ptr) { if (not arr_ptr->diff(state).empty()) { state.mutated_nodes_.push_back(dec_ptr); } diff --git a/dwave/optimization/src/nodes/lambda.cpp b/dwave/optimization/src/nodes/lambda.cpp index b40ec9a5..bf48885d 100644 --- a/dwave/optimization/src/nodes/lambda.cpp +++ b/dwave/optimization/src/nodes/lambda.cpp @@ -258,8 +258,7 @@ void AccumulateZipNode::initialize_state(State& state) const { ssize_t start_size = this->size(state); ssize_t num_args = operands_.size(); std::vector values; - State reg; - reg = expression_ptr_->empty_state(); + State reg = expression_ptr_->empty_state(); std::vector iterators; for (const ArrayNode* array_ptr : operands_) { From 423b6f4e716a8aa83678263c53586662deaa52d8 Mon Sep 17 00:00:00 2001 From: William Bernoudy Date: Mon, 10 Aug 2026 14:00:52 -0700 Subject: [PATCH 3/3] Small fixes for State after PR comments --- dwave/optimization/include/dwave-optimization/graph.hpp | 2 -- dwave/optimization/include/dwave-optimization/state.hpp | 6 +++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dwave/optimization/include/dwave-optimization/graph.hpp b/dwave/optimization/include/dwave-optimization/graph.hpp index 00ec336e..bbbef7bc 100644 --- a/dwave/optimization/include/dwave-optimization/graph.hpp +++ b/dwave/optimization/include/dwave-optimization/graph.hpp @@ -40,8 +40,6 @@ class Node; struct Decision {}; class Graph { - friend class State; - public: Graph() noexcept = default; ~Graph() noexcept = default; diff --git a/dwave/optimization/include/dwave-optimization/state.hpp b/dwave/optimization/include/dwave-optimization/state.hpp index 5e61239b..f3a703dd 100644 --- a/dwave/optimization/include/dwave-optimization/state.hpp +++ b/dwave/optimization/include/dwave-optimization/state.hpp @@ -18,6 +18,8 @@ #include #include +#include "dwave-optimization/common.hpp" + namespace dwave::optimization { // Generic base class for encoding the state of the model. In general, nodes @@ -41,7 +43,7 @@ class State { friend class Graph; public: - State(ssize_t size = 0) : node_data_(size) {} + State() {} template auto& operator[](index_type index) { @@ -58,6 +60,8 @@ class State { ssize_t size() const { return node_data_.size(); } private: + State(ssize_t size) : node_data_(size) {} + std::vector> node_data_; std::vector mutated_nodes_; };