Add method for removing redundant nodes

dwave/optimization/_model.pyi

@@ -71,6 +71,7 @@ class _Graph:
     def num_inputs(self) -> int: ...
     def num_nodes(self) -> int: ...
     def num_symbols(self) -> int: ...
+    def remove_redundant_symbols(self, time_limit_s: None | float = None) -> int: ...
     def remove_unused_symbols(self) -> int: ...
     def state_size(self) -> int: ...
     def unlock(self): ...

dwave/optimization/_model.pyx

@@ -928,6 +928,46 @@ cdef class _Graph:
         """
         return self.num_nodes()
 
+    def remove_redundant_symbols(self, time_limit_s=None):
+        """Remove redundant symbols from the model.
+
+        Symbols are redundant if they are the same type, they share the
+        same predecessors, and they encode the same operation.
+
+        Args:
+            time_limit_s (float): The maximum amount of time to spend trying
+                to remove redundant symbols. If not provided, the method will
+                run until there are no redundant symbols to remove.
+
+        Returns:
+            int: Number of symbols removed.
+
+        Examples:
+
+            >>> from dwave.optimization import Model
+            ...
+            >>> model = Model()
+            ...
+            >>> x = model.integer(5)
+            ...
+            >>> -(x + 1).sum()  # doctest: +ELLIPSIS
+            <dwave.optimization.symbols.unaryop.Negative at ...>
+            >>> -(x + 1).sum()  # doctest: +ELLIPSIS
+            <dwave.optimization.symbols.unaryop.Negative at ...>
+            >>> model.num_symbols()
+            8
+            >>> model.remove_redundant_symbols()
+            3
+            >>> model.num_symbols()
+            5
+        """
+        if self.is_locked():
+            raise ValueError("cannot remove symbols from a locked model")
+        if time_limit_s is None:
+            return self._graph.remove_redundant_nodes(False)
+        else:
+            return self._graph.remove_redundant_nodes(False, time_limit_s)
+
     def remove_unused_symbols(self):
         """Remove unused symbols from the model.
 
@@ -981,7 +1021,7 @@ cdef class _Graph:
         """
         if self.is_locked():
             raise ValueError("cannot remove symbols from a locked model")
-        return self._graph.remove_unused_nodes()
+        return self._graph.remove_unused_nodes(False)
 
     def state_size(self):
         r"""Return an estimate of the size, in bytes, of a model's state.

dwave/optimization/include/dwave-optimization/graph.hpp

@@ -156,6 +156,17 @@ class Graph {
     /// Reset the state of the given node and all successors recursively.
     static void recursive_reset(State& state, const Node* ptr);
 
+    /// Remove redundant nodes from the graph.
+    ///
+    /// Redundant nodes are ones that are Node::equal_to() another node in the
+    /// graph.
+    ///
+    /// Returns the number of nodes removed from the graph.
+    ssize_t remove_redundant_nodes(
+        bool ignore_listeners = false,
+        double time_limit_s = std::numeric_limits<double>::infinity()
+    );
+
     /// Remove unused nodes from the graph.
     ///
     /// This method will reset the topological sort if there is one.
@@ -263,6 +274,12 @@ class Node {
     /// derived from its predecessors. Defaults to `true`, except for decisions.
     virtual bool deterministic_state() const { return true; }
 
+    /// Test whether two nodes are equal. Each node class defines equality for
+    /// itself but nodes *must* share the same set of
+    /// predecessors (permutations are sometimes allowed) and they *must* be the
+    /// same type. Also that they can have false negatives in some cases.
+    virtual bool equal_to(const Node& rhs) const = 0;
+
     /// Return a shared pointer to a bool value. When the node is destructed
     /// the bool will be set to True
     std::shared_ptr<const bool> expired_ptr() const { return expired_ptr_; }
@@ -305,6 +322,11 @@ class Node {
     /// Return the successors of the node.
     const std::vector<SuccessorView>& successors() const { return successors_; }
 
+    /// Take the successors from `from` and make them successors of the node.
+    /// Note that this bypasses most checks! Use with extreme caution as it
+    /// make create undefined models.
+    void take_successors(Node& from);
+
     /// The current topological index. Will be negative if unsorted.
     ssize_t topological_index() const { return topological_index_; }
 
@@ -322,6 +344,7 @@ class Node {
     friend void Graph::reset_topological_sort();
     template <class NodeType, class... Args>
     friend NodeType* Graph::emplace_node(Args&&...);
+    friend ssize_t Graph::remove_redundant_nodes(bool, double);
     friend ssize_t Graph::remove_unused_nodes(bool);
 
  protected:
@@ -380,6 +403,10 @@ class Node {
     // Remove a successor. *Does not* remove itself from it's successor's predecessors.
     ssize_t remove_successor_(const Node* ptr);
 
+    // Replace the predecessor at `index` with the node specified by `node_ptr`.
+    // Does not update `node_ptr`.
+    virtual void replace_predecessor_(ssize_t index, Node* node_ptr);
+
  private:
     ssize_t topological_index_ = -1;  // negative is unset
 
@@ -431,6 +458,10 @@ class DecisionNode : public Decision, public virtual Node {
     /// Decision nodes by definition do not have a deterministic state.
     bool deterministic_state() const final { return false; }
 
+    /// Decision can only ever be equal to themselves because they are
+    /// independent variables.
+    bool equal_to(const Node& rhs) const final { return static_cast<const Node*>(this) == &rhs; }
+
     /// Decisions don't have predecessors so no one should be calling update().
     /// Always throws a logic_error.
     [[noreturn]] void update(State& state, int index) const override;
@@ -440,4 +471,44 @@ class DecisionNode : public Decision, public virtual Node {
     bool removable_() const override { return false; }
 };
 
+/// Provide an implementation of equal_to() that defers to a type-specific
+/// implementation.
+template<std::derived_from<Node> Base, typename Derived = void>
+struct EqualityMixin : Base {
+    /// @copydoc Node::equal_to()
+    bool equal_to(const Node& rhs) const final {
+        // Every node is equal with itself
+        if (&rhs == static_cast<const Node*>(this)) return true;
+
+        // A node cannot be equal if they are not of the same type.
+        const auto* rhs_ptr = dynamic_cast<const Derived*>(&rhs);
+        if (rhs_ptr == nullptr) return false;
+
+        // lhs and rhs are the same type, so let's go to our type-specific
+        // method.
+        return equal_to(*rhs_ptr);
+    }
+
+    // Nodes must implement `equal_to()` for their own type.
+    virtual bool equal_to(const Derived& rhs) const = 0;
+};
+
+/// Overload for EqualityMixin that does not require a class-specific equal_to()
+/// method overload. This simply checks that they have the same type and same
+/// predecessors.
+template<std::derived_from<Node> Base>
+struct EqualityMixin<Base, void> : Base {
+    /// @copydoc Node::equal_to()
+    bool equal_to(const Node& rhs) const final {
+        // Every node is equal with itself
+        if (&rhs == static_cast<const Node*>(this)) return true;
+
+        // Nodes that are not the same type cannot be equal
+        if (typeid(*this) != typeid(rhs)) return false;
+
+        // Nodes that are of the same type must have the same predecessors
+        return std::ranges::equal(this->predecessors(), rhs.predecessors());
+    }
+};
+
 }  // namespace dwave::optimization

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

@@ -29,14 +29,19 @@
 namespace dwave::optimization {
 
 template <class BinaryOp>
-class BinaryOpNode : public ArrayOutputMixin<ArrayNode> {
+class BinaryOpNode : public ArrayOutputMixin<EqualityMixin<ArrayNode, BinaryOpNode<BinaryOp>>> {
  public:
     // We need at least two nodes, and they must be the same shape
     BinaryOpNode(ArrayNode* a_ptr, ArrayNode* b_ptr);
 
     double const* buff(const State& state) const override;
     std::span<const Update> diff(const State& state) const override;
 
+    /// Two BinaryOpNodes are equal if they have the same operation and the
+    /// same predecessors. If the BinaryOp is commutative, then permutations
+    /// of predecessors are allowed.
+    bool equal_to(const BinaryOpNode& rhs) const override;
+
     /// @copydoc Array::integral()
     bool integral() const override;
 
@@ -46,10 +51,10 @@ class BinaryOpNode : public ArrayOutputMixin<ArrayNode> {
     /// @copydoc Array::max()
     double max() const override;
 
-    using ArrayOutputMixin::shape;
+    using ArrayOutputMixin<EqualityMixin<ArrayNode, BinaryOpNode<BinaryOp>>>::shape;
     std::span<const ssize_t> shape(const State& state) const override;
 
-    using ArrayOutputMixin::size;
+    using ArrayOutputMixin<EqualityMixin<ArrayNode, BinaryOpNode<BinaryOp>>>::size;
     ssize_t size(const State& state) const override;
 
     ssize_t size_diff(const State& state) const override;
@@ -63,20 +68,22 @@ class BinaryOpNode : public ArrayOutputMixin<ArrayNode> {
 
     // The predecessors of the operation, as Array*.
     std::span<Array* const> operands() {
-        assert(predecessors().size() == operands_.size());
+        assert(this->predecessors().size() == operands_.size());
         return operands_;
     }
     std::span<const Array* const> operands() const {
-        assert(predecessors().size() == operands_.size());
+        assert(this->predecessors().size() == operands_.size());
         return operands_;
     }
 
  private:
+    void replace_predecessor_(ssize_t previous_index, Node* node_ptr) override;
+
     BinaryOp op;
 
     // There are redundant, because we could dynamic_cast each time from
     // predecessors(), but this is more performant
-    std::array<Array* const, 2> operands_;
+    std::array<Array*, 2> operands_;
 
     const ValuesInfo values_info_;
     const SizeInfo sizeinfo_;

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

@@ -138,6 +138,9 @@ class DisjointBitSetNode : public ArrayOutputMixin<ArrayNode> {
 
     std::span<const Update> diff(const State& state) const override;
 
+    // A DisjointBitSetNode can only ever be equal to itself.
+    bool equal_to(const Node& rhs) const override;
+
     /// @copydoc Array::integral()
     bool integral() const override;
 
@@ -228,6 +231,9 @@ class DisjointListNode : public ArrayOutputMixin<ArrayNode> {
 
     std::span<const Update> diff(const State& state) const override;
 
+    // A DisjointListNode can only ever be equal to itself.
+    bool equal_to(const Node& rhs) const override;
+
     /// @copydoc Array::integral()
     bool integral() const override;
 

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

@@ -26,7 +26,7 @@
 namespace dwave::optimization {
 
 /// A contiguous block of numbers.
-class ConstantNode : public ArrayOutputMixin<ArrayNode> {
+class ConstantNode : public ArrayOutputMixin<EqualityMixin<ArrayNode, ConstantNode>> {
  public:
     struct DataSource {
         DataSource() = default;
@@ -100,6 +100,9 @@ class ConstantNode : public ArrayOutputMixin<ArrayNode> {
 
     // Overloads required by the Node ABC *************************************
 
+    /// @copydoc Node::equal_to()
+    bool equal_to(const ConstantNode& rhs) const override;
+
     // This node never needs to update its successors
     void propagate(State&) const noexcept override {}
 

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

@@ -24,7 +24,7 @@
 namespace dwave::optimization {
 
 /// A node encoding evenly spaced values within a given interval.
-class ARangeNode : public ArrayOutputMixin<ArrayNode> {
+class ARangeNode : public ArrayOutputMixin<EqualityMixin<ArrayNode, ARangeNode>> {
  public:
     using array_or_int = std::variant<const Array*, ssize_t>;
 
@@ -83,6 +83,9 @@ class ARangeNode : public ArrayOutputMixin<ArrayNode> {
     /// @copydoc Array::diff()
     std::span<const Update> diff(const State& state) const override;
 
+    /// ARangeNode is equal to another ARangeNode encoding the same range.
+    bool equal_to(const ARangeNode& rhs) const override;
+
     /// @copydoc Node::initialize_state()
     void initialize_state(State& state) const override;
 
@@ -126,10 +129,13 @@ class ARangeNode : public ArrayOutputMixin<ArrayNode> {
     /// The value or array defining the spacing between values.
     array_or_int step() const { return step_; }
 
+ protected:
+    void replace_predecessor_(ssize_t index, Node* node_ptr) override;
+
  private:
-    const array_or_int start_;
-    const array_or_int stop_;
-    const array_or_int step_;
+    array_or_int start_;
+    array_or_int stop_;
+    array_or_int step_;
 
     const std::pair<double, double> values_minmax_;
     const SizeInfo sizeinfo_;

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

@@ -26,7 +26,7 @@ namespace dwave::optimization {
 ///
 /// `condition` and `arr` must be the same size. This always outputs a
 /// 1d array.
-class ExtractNode : public ArrayOutputMixin<ArrayNode> {
+class ExtractNode : public ArrayOutputMixin<EqualityMixin<ArrayNode>> {
  public:
     ExtractNode(ArrayNode* condition_ptr, ArrayNode* arr_ptr);
 
@@ -73,6 +73,9 @@ class ExtractNode : public ArrayOutputMixin<ArrayNode> {
     /// @copydoc Array::sizeinfo()
     SizeInfo sizeinfo() const override;
 
+ protected:
+    void replace_predecessor_(ssize_t index, Node* node_ptr) override;
+
  private:
     // these are redundant, but convenient
     const Array* condition_ptr_;
@@ -87,13 +90,14 @@ class ExtractNode : public ArrayOutputMixin<ArrayNode> {
 /// `condition` must be either a scalar array or the same shape as `x` and `y`.
 /// `x` and `y` must have the same shape, including dynamic.
 /// dynamically sized `condition`s are not allowed.
-class WhereNode : public ArrayOutputMixin<ArrayNode> {
+class WhereNode : public ArrayOutputMixin<EqualityMixin<ArrayNode>> {
  public:
     WhereNode(ArrayNode* condition_ptr, ArrayNode* x_ptr, ArrayNode* y_ptr);
 
     double const* buff(const State& state) const override;
     void commit(State& state) const override;
     std::span<const Update> diff(const State& state) const override;
+
     void initialize_state(State& state) const override;
 
     /// @copydoc Array::integral()
@@ -116,6 +120,9 @@ class WhereNode : public ArrayOutputMixin<ArrayNode> {
     /// @copydoc Array::sizeinfo()
     SizeInfo sizeinfo() const override;
 
+ protected:
+    void replace_predecessor_(ssize_t index, Node* node_ptr) override;
+
  private:
     // these are redundant, but convenient
     const Array* condition_ptr_;