Skip to content

Enhancement/Sum constraints on NumberNode#471

Draft
fastbodin wants to merge 29 commits intodwavesystems:mainfrom
fastbodin:enhancement/axis_wise_bounds_numbernode
Draft

Enhancement/Sum constraints on NumberNode#471
fastbodin wants to merge 29 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.

fastbodin added 29 commits April 8, 2026 14:32
Data is stored at C++ level with the class `AxisBoundInfo` as private
attribute to `NumberNode`. Added relevant C++ tests.
For each bound axis and each hyperslice along said axis, we store the
running sum of the values within the hyperslice. This state dependant
data is stored via `NumberNodeStateData`. If `NumberNode` is initialized
with values, we check that all axis-wise bounds are satisfied.
Defined method to initialize_state() given exactly one axis-wise bound.
Fill state with lower bounds and increment until state satisfies
axis-wise bounds or determines infeasible. Added appropriate C++
IntegerNode and BinaryNode tests.
Made BoundAxisInfo and BoundAxisOperators members of NumberNode. Updated
all C++ tests. Optimized BufferIterator use in initialize_state().
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.
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.
Reflect version bump.
497 removed unset() and set() method. Removed them from
sum constraint C++ tests.
@fastbodin fastbodin force-pushed the enhancement/axis_wise_bounds_numbernode branch from b27eecc to 2ab16fc Compare April 8, 2026 21:39
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