Skip to content

Enhancement/Sum constraints on NumberNode#471

Open
fastbodin wants to merge 35 commits intodwavesystems:mainfrom
fastbodin:enhancement/axis_wise_bounds_numbernode
Open

Enhancement/Sum constraints on NumberNode#471
fastbodin wants to merge 35 commits intodwavesystems:mainfrom
fastbodin:enhancement/axis_wise_bounds_numbernode

Conversation

@fastbodin
Copy link
Copy Markdown
Contributor

@fastbodin fastbodin commented Jan 30, 2026

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

@fastbodin 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
Copy link
Copy Markdown
Contributor Author

Feature is fully implemented.

@fastbodin fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from e3b0255 to 1c177b4 Compare February 4, 2026 00:02
@fastbodin fastbodin marked this pull request as ready for review February 4, 2026 20:24
@fastbodin
Copy link
Copy Markdown
Contributor Author

fastbodin commented Feb 4, 2026

Closes #216.

@arcondello arcondello requested a review from wbernoudy February 4, 2026 22:49
@arcondello arcondello added the enhancement New feature or request label Feb 4, 2026
@fastbodin
Copy link
Copy Markdown
Contributor Author

First round of comments addressed.

@fastbodin fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from 4b2205c to c6a93d5 Compare February 5, 2026 06:37
@wbernoudy
Copy link
Copy Markdown
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 numbers.hpp on what that term means here.

@fastbodin fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from c6a93d5 to 72c6d96 Compare February 6, 2026 02:05
@fastbodin
Copy link
Copy Markdown
Contributor Author

Second round of comments addressed.

Copy link
Copy Markdown
Member

@arcondello arcondello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just two aesthetic comments

@fastbodin
Copy link
Copy Markdown
Contributor Author

Third round of comments addressed.

@fastbodin fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from a92e2bd to 116d03b Compare February 11, 2026 20:36
@fastbodin fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from f2b1cc4 to 3b10b41 Compare February 12, 2026 23:12
@fastbodin
Copy link
Copy Markdown
Contributor Author

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.

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.
`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`.
Naming convention change to methods, variables, and comments.
Reflect version bump.
@fastbodin fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from 30bb86c to 3302fd4 Compare March 31, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider adding the ability to specify one axis bound to BinaryVariable

3 participants