Enhancement/Sum constraints on NumberNode#471
Open
fastbodin wants to merge 35 commits intodwavesystems:mainfrom
Open
Enhancement/Sum constraints on NumberNode#471fastbodin wants to merge 35 commits intodwavesystems:mainfrom
fastbodin wants to merge 35 commits intodwavesystems:mainfrom
Conversation
bebd145 to
15f7b67
Compare
Contributor
Author
|
Feature is fully implemented. |
e3b0255 to
1c177b4
Compare
Contributor
Author
|
Closes #216. |
arcondello
reviewed
Feb 4, 2026
dwave/optimization/include/dwave-optimization/nodes/numbers.hpp
Outdated
Show resolved
Hide resolved
wbernoudy
reviewed
Feb 4, 2026
dwave/optimization/include/dwave-optimization/nodes/numbers.hpp
Outdated
Show resolved
Hide resolved
dwave/optimization/include/dwave-optimization/nodes/numbers.hpp
Outdated
Show resolved
Hide resolved
Contributor
Author
|
First round of comments addressed. |
4b2205c to
c6a93d5
Compare
wbernoudy
reviewed
Feb 5, 2026
wbernoudy
reviewed
Feb 5, 2026
Member
|
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 |
wbernoudy
reviewed
Feb 5, 2026
wbernoudy
reviewed
Feb 5, 2026
wbernoudy
reviewed
Feb 5, 2026
wbernoudy
reviewed
Feb 5, 2026
c6a93d5 to
72c6d96
Compare
Contributor
Author
|
Second round of comments addressed. |
arcondello
approved these changes
Feb 6, 2026
Member
arcondello
left a comment
There was a problem hiding this comment.
Looks great! Just two aesthetic comments
dwave/optimization/include/dwave-optimization/nodes/numbers.hpp
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Third round of comments addressed. |
wbernoudy
approved these changes
Feb 6, 2026
wbernoudy
reviewed
Feb 11, 2026
a92e2bd to
116d03b
Compare
wbernoudy
reviewed
Feb 12, 2026
f2b1cc4 to
3b10b41
Compare
Contributor
Author
|
Previously, users could not define a bound for the sum of the values over an entire |
Improved comments. Improved methods. Cleaned up C++ tests. Added static_casts where necessary for CircleCI.
When constructing a state that satisfies exactly one axis-wise bound.
Needed for Cython interface. Changed relevant C++ code. Used aliases in C++ NumberNode tests.
Also made BoundAxisOperaters an `enum` as opposed to `enum class` because we found a Cython workaround.
Resolved conflict in rebase.
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.
Removed `asserts()` that axis-wise bounds were satisfied to `NumberNode::propagate()`.
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.
In previous commits, we expanded the functionality of `NumberNode` such that a user may define implicit constraints on the sum of values within the node. We allow constraints on the entire array and along a fixed axis. The naming convention axis-wise bounds was given before we allowed constraints over the entire array. In the latter case, there is no `axis`. Changing naming convention to `sum constraints`.
See prior commit message.
Naming convention change to methods, variables, and comments.
Reflect version bump.
30bb86c to
3302fd4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Do not merge. Draft PR.
Only C++ has been implemented.Simply checking CircleCi.