Address 1st rnd. comments NumberNode axis-wise bounds

Author: fastbodin

dwave/optimization/include/dwave-optimization/nodes/numbers.hpp

@@ -144,7 +144,8 @@ class NumberNode : public ArrayOutputMixin<ArrayNode>, public DecisionNode {
     const std::vector<AxisBound>& axis_wise_bounds() const;
 
     /// Return the state-dependent sum of the values within each hyperslice
-    /// along each bound axis.
+    /// along each bound axis. The returned vector is indexed by the
+    /// bound axes in the same ordering that `axis_wise_bounds()` returns.
     const std::vector<std::vector<double>>& bound_axis_sums(State& state) const;
 
  protected:

dwave/optimization/libcpp/nodes/numbers.pxd

@@ -30,10 +30,10 @@ cdef extern from "dwave-optimization/nodes/numbers.hpp" namespace "dwave::optimi
                 GreaterEqual
 
             AxisBound(Py_ssize_t axis, vector[Operator] axis_opertors, 
-                          vector[double] axis_bounds)
+                      vector[double] axis_bounds)
             Py_ssize_t axis
-            vector[Operator] operators;
-            vector[double] bounds;
+            vector[Operator] operators
+            vector[double] bounds
 
         void initialize_state(State&, vector[double]) except+
         double lower_bound(Py_ssize_t index)

dwave/optimization/model.py

@@ -166,7 +166,8 @@ def objective(self, value: ArraySymbol):
     def binary(self, shape: None | _ShapeLike = None,
                lower_bound: None | np.typing.ArrayLike = None,
                upper_bound: None | np.typing.ArrayLike = None,
-               subject_to: None | np.typing.ArrayLike = None) -> BinaryVariable:
+               subject_to: None | list[tuple(int, str | list[str], float |
+                                       list[float])] = None) -> BinaryVariable:
         r"""Create a binary symbol as a decision variable.
 
         Args:
@@ -183,13 +184,13 @@ def binary(self, shape: None | _ShapeLike = None,
                 array of tuples where each tuple has the form: (axis, operators, bounds)
                 - axis (int): The axis along which the bounds are applied.
                 - operators (str | array[str]): The operator(s) ("<=", "==", or ">="). 
-                A single operator applies to all hyperslices along the axis; an
-                array specifies one operator per hyperslice.
+                A single operator applies to all slices along the axis; an
+                array specifies one operator per slice.
                 - bounds (float | array[float]): The bound value(s). A single value
-                applies to all hyperslices; an array specifies one bound per hyperslice.
-                If provided, the sum of values within each hyperslice along the specified
-                axis must satisfy the corresponding operator–bound pair.
-                Note: At most one axis-wise bound may be provided.
+                applies to all slices; an array specifies one bound per slice.
+                If provided, the sum of values within each slice along the
+                specified axis must satisfy the corresponding operator–bound
+                pair. Note: At most one axis-wise bound may be provided.
 
         Returns:
             A binary symbol.
@@ -230,13 +231,13 @@ def binary(self, shape: None | _ShapeLike = None,
             This example adds a :math:`(2x3)`-sized binary symbol with
             index-wise lower bounds and an axis-wise bound along axis 1. Let
             x_i (int i : 0 <= i <= 2) denote the sum of the values within
-            hyperslice i along axis 1. For each state defined for this symbol:
+            slice i along axis 1. For each state defined for this symbol:
             (x_0 <= 0), (x_1 == 2), and (x_2 >= 1).
 
             >>> from dwave.optimization.model import Model
             >>> import numpy as np
             >>> model = Model()
-            >>> n = model.binary([2, 3], lower_bound=[[0, 1, 1], [0, 1, 0]],
+            >>> b = model.binary([2, 3], lower_bound=[[0, 1, 1], [0, 1, 0]],
             ... subject_to=[(1, ["<=", "==", ">="], [0, 2, 1])])
             >>> np.all(n.axis_wise_bounds() == [(1, ["<=", "==", ">="], [0, 2, 1])])
             True
@@ -508,8 +509,8 @@ def integer(
             shape: None | _ShapeLike = None,
             lower_bound: None | numpy.typing.ArrayLike = None,
             upper_bound: None | numpy.typing.ArrayLike = None,
-            subject_to: None | np.typing.ArrayLike = None
-            ) -> IntegerVariable:
+               subject_to: None | list[tuple(int, str | list[str], float |
+                                       list[float])] = None) -> IntegerVariable:
         r"""Create an integer symbol as a decision variable.
 
         Args:
@@ -526,13 +527,13 @@ def integer(
                 array of tuples where each tuple has the form: (axis, operators, bounds)
                 - axis (int): The axis along which the bounds are applied.
                 - operators (str | array[str]): The operator(s) ("<=", "==", or ">="). 
-                A single operator applies to all hyperslices along the axis; an
-                array specifies one operator per hyperslice.
+                A single operator applies to all slice along the axis; an array
+                specifies one operator per slice.
                 - bounds (float | array[float]): The bound value(s). A single value
-                applies to all hyperslices; an array specifies one bound per hyperslice.
-                If provided, the sum of values within each hyperslice along the specified
-                axis must satisfy the corresponding operator–bound pair.
-                Note: At most one axis-wise bound may be provided.
+                applies to all slices; an array specifies one bound per slice.
+                If provided, the sum of values within each slice along the
+                specified axis must satisfy the corresponding operator–bound
+                pair. Note: At most one axis-wise bound may be provided.
 
         Returns:
             An integer symbol.
@@ -574,7 +575,7 @@ def integer(
             This example adds a :math:`(2x3)`-sized integer symbol with general
             lower and upper bounds and an axis-wise bound along axis 1. Let x_i
             (int i : 0 <= i <= 2) denote the sum of the values within
-            hyperslice i along axis 1. For each state defined for this symbol:
+            slice i along axis 1. For each state defined for this symbol:
             (x_0 <= 2), (x_1 <= 4), and (x_2 <= 5).
 
             >>> from dwave.optimization.model import Model

dwave/optimization/src/nodes/numbers.cpp

@@ -177,19 +177,18 @@ void NumberNode::initialize_state(State& state, std::vector<double>&& number_dat
 
     if (bound_axes_info_.size() == 0) {  // No bound axes to consider.
         emplace_data_ptr<NumberNodeStateData>(state, std::move(number_data));
-        return;
-    }
+    } else {
+        // Given the assingnment to NumberNode `number_data`, compute the sum of the
+        // values within each hyperslice along each bound axis.
+        std::vector<std::vector<double>> bound_axes_sums = get_bound_axes_sums(this, number_data);
 
-    // Given the assingnment to NumberNode `number_data`, compute the sum of the
-    // values within each hyperslice along each bound axis.
-    std::vector<std::vector<double>> bound_axes_sums = get_bound_axes_sums(this, number_data);
+        if (!satisfies_axis_wise_bounds(bound_axes_info_, bound_axes_sums)) {
+            throw std::invalid_argument("Initialized values do not satisfy axis-wise bounds.");
+        }
 
-    if (!satisfies_axis_wise_bounds(bound_axes_info_, bound_axes_sums)) {
-        throw std::invalid_argument("Initialized values do not satisfy axis-wise bounds.");
+        emplace_data_ptr<NumberNodeStateData>(state, std::move(number_data),
+                                              std::move(bound_axes_sums));
     }
-
-    emplace_data_ptr<NumberNodeStateData>(state, std::move(number_data),
-                                          std::move(bound_axes_sums));
 }
 
 /// Given a `span` (used for strides or shape data), reorder the values
@@ -282,8 +281,8 @@ void construct_state_given_exactly_one_bound_axis(const NumberNode* node,
     const std::vector<ssize_t> buff_strides = shift_axis_data(node->strides(), bound_axis);
     // Define an iterator for `values` corresponding with the beginning of
     // slice 0 along the bound axis.
-    BufferIterator<double, double, false> slice_0_it(values.data(), ndim, buff_shape.data(),
-                                                     buff_strides.data());
+    const BufferIterator<double, double, false> slice_0_it(values.data(), ndim, buff_shape.data(),
+                                                           buff_strides.data());
     // Determine the size of each hyperslice along the bound axis.
     const ssize_t slice_size = std::accumulate(buff_shape.begin() + 1, buff_shape.end(), 1.0,
                                                std::multiplies<ssize_t>());
@@ -336,14 +335,12 @@ void NumberNode::initialize_state(State& state) const {
             values.push_back(default_value(i));
         }
         initialize_state(state, std::move(values));
-        return;
     } else if (bound_axes_info_.size() == 1) {
         construct_state_given_exactly_one_bound_axis(this, values);
         initialize_state(state, std::move(values));
-        return;
+    } else {
+        unreachable();
     }
-
-    throw std::invalid_argument("Cannot initialize state with multiple bound axes.");
 }
 
 void NumberNode::commit(State& state) const noexcept {

dwave/optimization/symbols/numbers.pyx

@@ -16,6 +16,7 @@
 
 import json
 
+import collections.abc
 import numpy as np
 
 from cython.operator cimport typeid
@@ -71,25 +72,20 @@ cdef vector[NumberNode.AxisBound] _convert_python_bound_axes(
         if not isinstance(axis, int):
             raise TypeError("Bound axis must be an int.")
 
-        cpp_ops.clear()
         if isinstance(py_ops, str):
+            cpp_ops.resize(1)
             # One operator defined for all slices.
-            cpp_ops.push_back(_parse_python_operator(py_ops))
-        else:
+            cpp_ops[0] = _parse_python_operator(py_ops)
+        elif isinstance(py_ops, collections.abc.Iterable):
             # Operator defined per slice.
-            ops_array = np.asarray(py_ops, order='C')
-            if (ops_array.ndim <= 1):
-                cpp_ops.reserve(ops_array.size)
-                for op in ops_array:
-                    # Convert op to `str` because _parse_python_operator()
-                    # does not expect a `numpy.str_`.
-                    cpp_ops.push_back(_parse_python_operator(str(op)))
-            else:
-                raise TypeError("Bound axis operator(s) should be str or"
-                                " a 1D-array of str(s).")
+            cpp_ops.reserve(len(py_ops))
+            for op in py_ops:
+                cpp_ops.push_back(_parse_python_operator(op))
+        else:
+            raise TypeError("Bound axis operator(s) should be str or a 1D-array"
+                            " of str(s).")
 
-        cpp_bounds.clear()
-        bound_array = np.asarray_chkfinite(py_bounds, dtype=np.double, order='C')
+        bound_array = np.asarray_chkfinite(py_bounds, dtype=np.double)
         if (bound_array.ndim <= 1):
             mem = bound_array.ravel()
             cpp_bounds.reserve(mem.shape[0])
@@ -98,7 +94,7 @@ cdef vector[NumberNode.AxisBound] _convert_python_bound_axes(
         else:
             raise TypeError("Bound axis bound(s) should be scalar or 1D-array.")
 
-        output.push_back(NumberNode.AxisBound(axis, cpp_ops, cpp_bounds))
+        output.push_back(NumberNode.AxisBound(axis, move(cpp_ops), move(cpp_bounds)))
 
     return output
 
@@ -206,10 +202,9 @@ cdef class BinaryVariable(ArraySymbol):
             subject_to = None
         else:
             with zf.open(info, "r") as f:
-                subject_to = json.load(f)
                 # Note that import is a list of lists, not a list of tuples.
                 # Hence we convert to tuple. We could also support lists.
-                subject_to = [(axis, ops, bounds) for axis, ops, bounds in subject_to]
+                subject_to = [(axis, ops, bounds) for axis, ops, bounds in json.load(f)]
 
         return BinaryVariable(model,
                               shape=shape_info["shape"],
@@ -415,11 +410,9 @@ cdef class IntegerVariable(ArraySymbol):
             subject_to = None
         else:
             with zf.open(info, "r") as f:
-                # Note that import is a list of lists, not a list of tuples
-                subject_to = json.load(f)
                 # Note that import is a list of lists, not a list of tuples.
                 # Hence we convert to tuple. We could also support lists.
-                subject_to = [(axis, ops, bounds) for axis, ops, bounds in subject_to]
+                subject_to = [(axis, ops, bounds) for axis, ops, bounds in json.load(f)]
 
         return IntegerVariable(model,
                                shape=shape_info["shape"],

tests/test_symbols.py

@@ -742,7 +742,7 @@ def test_axis_wise_bounds(self):
         self.assertEqual(x.axis_wise_bounds(), [(0, ["<=", "=="], [1])])
         x = model.binary((2, 3), subject_to=[(0, "<=", 1)])
         self.assertEqual(x.axis_wise_bounds(), [(0, ["<="], [1])])
-        x = model.binary((2, 3), subject_to=[(0, np.asarray(["<=", "=="]), np.asarray([1, 2]))])
+        x = model.binary((2, 3), subject_to=[(0, ["<=", "=="], np.asarray([1, 2]))])
         self.assertEqual(x.axis_wise_bounds(), [(0, ["<=", "=="], [1, 2])])
 
         # infeasible axis-wise bounds
@@ -1924,7 +1924,7 @@ def test_axis_wise_bounds(self):
         self.assertEqual(x.axis_wise_bounds(), [(0, ["<=", "=="], [1])])
         x = model.integer((2, 3), subject_to=[(0, "<=", 1)])
         self.assertEqual(x.axis_wise_bounds(), [(0, ["<="], [1])])
-        x = model.integer((2, 3), subject_to=[(0, np.asarray(["<=", "=="]), np.asarray([1, 2]))])
+        x = model.integer((2, 3), subject_to=[(0, ["<=", "=="], np.asarray([1, 2]))])
         self.assertEqual(x.axis_wise_bounds(), [(0, ["<=", "=="], [1, 2])])
 
         # infeasible axis-wise bounds
@@ -1953,6 +1953,10 @@ def test_axis_wise_bounds(self):
         with self.assertRaises(TypeError):
             model.integer((2, 3), subject_to=[(1, [["=="]], [0, 0, 0])])
 
+        # invalid number of bound axes
+        with self.assertRaises(ValueError):
+            model.integer((2, 3), subject_to=[(0, "==", 1), (1, "<=", [1, 1, 1])])
+
     # Todo: we can generalize many of these tests for all decisions that can have
     # their state set