Enhancement/Sum constraints on NumberNode by fastbodin · Pull Request #471 · dwavesystems/dwave-optimization

fastbodin · 2026-01-30T22:42:05Z

Do not merge. Draft PR. ~~Only C++ has been implemented.~~ Simply checking CircleCi.

fastbodin · 2026-02-03T23:45:52Z

Feature is fully implemented.

fastbodin · 2026-02-04T20:32:33Z

Closes #216.

fastbodin · 2026-02-05T00:53:48Z

First round of comments addressed.

wbernoudy · 2026-02-05T22:12:11Z

I do see "hyperslice" used in a lot of comments. Would prefer that is changed to just "slice" as well, or at least a small comment maybe in the header numbers.hpp on what that term means here.

fastbodin · 2026-02-06T02:07:44Z

Second round of comments addressed.

arcondello

Looks great! Just two aesthetic comments

fastbodin · 2026-02-06T22:34:43Z

Third round of comments addressed.

fastbodin · 2026-02-27T20:57:53Z

Previously, users could not define a bound for the sum of the values over an entire NumberNode array. This lead to the undesired behaviour that users could not define a bound on the sum of the values in a NumberNode vector.

Previously accepted list of tuples and list of lists. Now simply list of tuples for consistency.

Specific to axis-wise bounds.

`BoundAxisInfo` -> `AxisBound` and `BoundAxisOperator` -> `Operator`. `Operator` is now a nested enum classs of `AxisBound`.

Added indicator variable that all bound axis operators are `==` to reduce redundancy in `NumberNode::exchange()` method.

Made axis, operators, and bounds private members. Added axis(), num_bounds(), and num_operators() methods. Updated C++ code/tests, Python, and Cython to reflect this.

Added a `const` to `State& state`. Fixed typos.

Previously, users could not define a bound for the sum of the values over an entire `NumberNode` array. This lead to the undesired behaviour that users could not define a bound on the sum of the values in a `NumberNode` vector.

Users may now define a bound on the sum of the values in the entire `NumberNode` array.

See prior commit message.

Reflect version bump.

497 removed unset() and set() method. Removed them from sum constraint C++ tests.

Simplify logic in `NumberNode` methods. In particular, methods that handle sum constraints where `axis` is optional. Moved `commit()`, `revert()`, and `update_sum_constraint_lhs()` onto `NumberNodeStateData`. Added a slice cache for faster `reverts()` Deleted `prior_sum_constraint` (unnecessary due to slice cache). Remove `NumberNode::sum_constraints_all_equals_`. Unnecessary given slice cache. Update `NumberNodeStateData` and mutate methods to reflect change. Remove excessive static_casts in NumberNode C++. Users may now provide the slices (per sum constraint) that a given index lies on. This is relevant to `exchange()`, `set_value()`, `clip_and_set_value()`, and `flip()`.

wbernoudy · 2026-04-15T17:47:21Z

        return std::make_unique<NumberNodeStateData>(*this);
    }

+    /// Commit the state dependant data of NumberNode.


dependent (also in a few other places)

wbernoudy · 2026-04-15T17:56:50Z

+    assert(difference != 0);              // Should not call when no change occurs.
+    assert(sum_constraints.size() == sum_constraints_lhs.size());
+
+    if (slices.has_value()) {


Seeing that you have separate implementations for the two cases of slices being provided or not, did you consider making overloads for the these methods instead of having default values for slices? i.e. make overloads that take std::vector<ssize_t> slices as the final argument.

Yes. I flip-flopped on that. Happy to make the change 👍

Fixed in dbaa0ec

Previously, used `std::optional<>` to capture both cases. Updated all relevant `NumberNode` mutate methods to handle the overload.

fastbodin · 2026-04-15T20:33:52Z

Added custom BinaryNode state data to track indices (per sum constraint and per slice) whose buffer value is 1 and 0 via DisjointSparseSet

wbernoudy · 2026-04-15T22:48:52Z

+        }
+
+        /// Return the `i`th index in `dense_sets_[dense_set]` whose buffer value is 0.
+        ssize_t get_ith_false_index(const ssize_t dense_id, const ssize_t i) const {


If I call this immediately after construction, it will return -1 correct? Do I need to call add_index() for each index before I am able to use this method?

Correct. There are assert to catch this in update_true() and update_false().

Would it be possible to set up the dense sets on construction so that's not possible?

Definitely possible. I did it this way to make BinaryNodeStateData::compute_slice_indices_() simple. I could hand buffer iterators per slice to each dense set.... It will take some thinking to do it nicely.

I just realized that I did not answer your second question. You do not need to call add_index() for each index before you are able to use get_ith_false_index(). When you add indices it emplace_back() them on their appropriate dense set.

The only methods that you could not use are update_false() and update_true(), since you cannot update something that has not been added.

Given that num_true(), num_false(), get_ith_true_index() and get_ith_false_index() all work even when all indices have not been added, I am inclined to leave it as is. An index should only ever call update_true() or update_false() when it has already been added.

wbernoudy · 2026-04-15T22:59:14Z

+
+    /// Given a slice (w.r.t a sum constraint), return the `i`th index in in
+    /// the slice whose buffer value is 1.
+    ssize_t get_ith_true_index(const ssize_t sum_constraint_id, const ssize_t slice,


I would consider making slice_indices_ public and having BinaryNode call these methods directly on the DisjointSparseSets instead of having more pass through methods

I went back-and-forth on that. Happy to make the change

Was there a downside to exposing them publicly?

I don't think so. At first I thought it would be best practice to have them as pass through methods so that they handle their own reporting internally. But then, after having done it, the number of pass throughs felt excessive 🤷 . The fact that you feel the same suggests I should change it 👍

wbernoudy · 2026-04-15T23:06:26Z

+            // Reverse the change applied to each slice.
+            for (size_t j = 0, stop = slices.size(); j < stop; ++j) {
+                sum_constraints_lhs[j][slices[j]] -= difference;
+                slice_indices_[j].update(update.index, slices[j], -difference);


The definition of update() and calling it with the difference here feels confusing to me, could you instead just pass in the value (update.value) into update() and have it take true/false? Or just

if (update.value) slice_indices_[j].update_true(update.index, slices[j]); else slice_indices_[j].update_false(update.index, slices[j]);

I did it to mirror updates to sum_constraint_lhs. Agreed it feels awkward. Happy to change 👍

`BinaryNodeStateData::slice_indices_` is used to cache, per sum constraint, the flat indices of each slice. For a given sum constraint and slice, the cached indices are ordered such that the indices in the region `[0, num_true(slice))` have buffer value 1 and the indices in the region `[num_true(slice), end)` have buffer value 0. The data structure used is similar to a `SparseSet` but is generalized to a `DisjointSparseSet` where there is one `sparse` look-up table but multiple disjoint `dense` data tables. All relevant `BinaryNode` methods have changed to reflect updating and reverting this data.

fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch 2 times, most recently from bebd145 to 15f7b67 Compare February 3, 2026 23:11

fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from e3b0255 to 1c177b4 Compare February 4, 2026 00:02

fastbodin marked this pull request as ready for review February 4, 2026 20:24

arcondello linked an issue Feb 4, 2026 that may be closed by this pull request

Consider adding the ability to specify one axis bound to BinaryVariable #216

Open

arcondello reviewed Feb 4, 2026

View reviewed changes

Comment thread dwave/optimization/include/dwave-optimization/nodes/numbers.hpp Outdated

arcondello requested a review from wbernoudy February 4, 2026 22:49

arcondello added the enhancement New feature or request label Feb 4, 2026

wbernoudy reviewed Feb 4, 2026

View reviewed changes

fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from 4b2205c to c6a93d5 Compare February 5, 2026 06:37