Make State a class and add Graph::mutated() - #589
Conversation
| struct Decision {}; | ||
|
|
||
| class Graph { | ||
| friend class State; |
| return node_data_[index]; | ||
| } | ||
|
|
||
| void resize(ssize_t size) { node_data_.resize(size); } |
There was a problem hiding this comment.
Since we're granting friendship to Graph, do we want to make this private?
There was a problem hiding this comment.
Looks like this is used by the cython as well (e.g. https://github.com/dwavesystems/dwave-optimization/blob/master/dwave/optimization/states.pyx#L305). Perhaps we could change the behavior of Graph::initialize_state(State&) to resize the state if the given size is zero?
There was a problem hiding this comment.
Ok, I am fine to leave this here then.
| } | ||
|
|
||
| std::span<const DecisionNode*> Graph::mutated(State& state) const { | ||
| // We will want to eventually replace this implementation with an approach where |
There was a problem hiding this comment.
IMO we want to do this as part of this PR. Probably means keeping a boolean array of flags as well to determine which decisions have already been added?
| friend class Graph; | ||
|
|
||
| public: | ||
| State(ssize_t size = 0) : node_data_(size) {} |
There was a problem hiding this comment.
Do we want to make the constructors private since we're granting friendship to Graph? We might want to leave the default (empty) constructor public for Cython reasons.
There was a problem hiding this comment.
Yes, makes sense to me, I will split the empty constructor out.
|
|
||
| void resize(ssize_t size) { node_data_.resize(size); } | ||
|
|
||
| ssize_t size() const { return node_data_.size(); } |
There was a problem hiding this comment.
Need to include dwave-optimization/common.hpp for ssize_t, this will fix the current windows CI failures.
07ce407 to
423b6f4
Compare
2 participants
What does this implement/fix?
This PR adds the
Graph::mutated()method which returns the list of decision nodes with "pending" changes (see docstring for more details). It also makesStatea class rather than an alias to a vector of data pointers in order to help implementGraph::mutated().Additional information
For now, I left the implementation of
Graph::mutated()simple, and it will return only decision nodes, and not specific successors ofDisjointListsNodeandDisjointBitSetsNode. However, if we plan to use this directly withdescendants()/propagate()etc., we may need to consider returning specific successors because of the known performance hit on models with large amounts of lists/sets on individualDisjointListsNode/DisjointBitSetsNodes.AI Generation Disclosure
No AI tools used