PR review medium effort fixes: deduplicate canonicalizers and optimize bounds (#144)

- Add _common.py with canonicalize_unary_smooth() helper to reduce code
  duplication across hyperbolic, trig, exp, and prod canonicalizers
- Refactor canonicalizer files to use the common helper
- Preallocate numpy arrays in Bounds class instead of list extend + convert
- Fix hessian() to return typed empty array (dtype=np.float64)
- Cache empty hessian structure in hessianstructure() for quasi-Newton

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: Transurgeon

cvxpy/reductions/dnlp2smooth/canonicalizers/_common.py

@@ -0,0 +1,67 @@
+"""
+Copyright 2025 CVXPY developers
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+"""
+
+import numpy as np
+
+from cvxpy.expressions.variable import Variable
+
+
+def canonicalize_unary_smooth(expr, args, bounds=None, default_value=None):
+    """
+    Generic canonicalization for smooth unary functions.
+
+    Ensures the argument is a Variable. If already a Variable (with no bounds
+    constraint), returns unchanged. Otherwise, creates a new Variable with
+    an equality constraint to the original argument.
+
+    Parameters
+    ----------
+    expr : Atom
+        The expression being canonicalized.
+    args : list
+        The canonicalized arguments of expr (expects single argument).
+    bounds : tuple or None
+        Optional (lower, upper) bounds for the new Variable.
+    default_value : array-like or None
+        Default value to use if arg has no value. If None, uses arg.value.
+
+    Returns
+    -------
+    tuple
+        (canonicalized_expr, list_of_constraints)
+    """
+    arg = args[0]
+
+    # If already a Variable with no bounds requirement, return unchanged
+    if isinstance(arg, Variable) and bounds is None:
+        return expr.copy([arg]), []
+
+    # Create new Variable, optionally with bounds
+    if bounds is not None:
+        t = Variable(arg.shape, bounds=bounds)
+    else:
+        t = Variable(arg.shape)
+
+    # Set initial value
+    if arg.value is not None:
+        t.value = arg.value
+    elif default_value is not None:
+        if callable(default_value):
+            t.value = default_value(arg.shape)
+        else:
+            t.value = np.broadcast_to(default_value, arg.shape)
+
+    return expr.copy([t]), [t == arg]

cvxpy/reductions/dnlp2smooth/canonicalizers/exp_canon.py

@@ -13,14 +13,9 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 """
-from cvxpy.expressions.variable import Variable
+
+from cvxpy.reductions.dnlp2smooth.canonicalizers._common import canonicalize_unary_smooth
 
 
 def exp_canon(expr, args):
-    if isinstance(args[0], Variable):
-        return expr.copy([args[0]]), []
-    else:
-        t = Variable(args[0].shape)
-        if args[0].value is not None:
-            t.value = args[0].value
-        return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args)

cvxpy/reductions/dnlp2smooth/canonicalizers/hyperbolic_canon.py

@@ -13,38 +13,21 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 """
-from cvxpy.expressions.variable import Variable
+
+from cvxpy.reductions.dnlp2smooth.canonicalizers._common import canonicalize_unary_smooth
 
 
 def sinh_canon(expr, args):
-    if isinstance(args[0], Variable):
-        return expr, []
-    else:
-        t = Variable(args[0].shape)
-        if args[0].value is not None:
-            t.value = args[0].value
-        return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args)
+
 
 def tanh_canon(expr, args):
-    if isinstance(args[0], Variable):
-        return expr, []
-    else:
-        t = Variable(args[0].shape)
-        if args[0].value is not None:
-            t.value = args[0].value
-        return expr.copy([t]), [t == args[0]]
-    
+    return canonicalize_unary_smooth(expr, args)
+
+
 def asinh_canon(expr, args):
-    if isinstance(args[0], Variable):
-        return expr, []
-    else:
-        t = Variable(args[0].shape)
-        if args[0].value is not None:
-            t.value = args[0].value
-        return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args)
+
 
 def atanh_canon(expr, args):
-    t = Variable(args[0].shape, bounds=[-1, 1])
-    if args[0].value is not None:
-        t.value = args[0].value
-    return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args, bounds=(-1, 1))

cvxpy/reductions/dnlp2smooth/canonicalizers/prod_canon.py

@@ -16,7 +16,7 @@
 
 import numpy as np
 
-from cvxpy.expressions.variable import Variable
+from cvxpy.reductions.dnlp2smooth.canonicalizers._common import canonicalize_unary_smooth
 
 
 def prod_canon(expr, args):
@@ -26,14 +26,4 @@ def prod_canon(expr, args):
     Since prod is a smooth function with implemented gradients,
     we simply ensure the argument is a Variable.
     """
-    if isinstance(args[0], Variable):
-        return expr.copy([args[0]]), []
-
-    t = Variable(args[0].shape)
-
-    if args[0].value is not None:
-        t.value = args[0].value
-    else:
-        t.value = np.ones(args[0].shape)
-
-    return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args, default_value=np.ones)

cvxpy/reductions/dnlp2smooth/canonicalizers/trig_canon.py

@@ -13,29 +13,19 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 """
-from cvxpy.expressions.variable import Variable
+
+import numpy as np
+
+from cvxpy.reductions.dnlp2smooth.canonicalizers._common import canonicalize_unary_smooth
 
 
 def sin_canon(expr, args):
-    if isinstance(args[0], Variable):
-        return expr, []
-    else:
-        t = Variable(args[0].shape)
-        if args[0].value is not None:
-            t.value = args[0].value
-        return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args)
+
 
 def cos_canon(expr, args):
-    if isinstance(args[0], Variable):
-        return expr, []
-    else:
-        t = Variable(args[0].shape)
-        if args[0].value is not None:
-            t.value = args[0].value
-        return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args)
+
 
 def tan_canon(expr, args):
-    t = Variable(args[0].shape, bounds=[-3.14159/2, 3.14159/2])
-    if args[0].value is not None:
-        t.value = args[0].value
-    return expr.copy([t]), [t == args[0]]
+    return canonicalize_unary_smooth(expr, args, bounds=(-np.pi / 2, np.pi / 2))

cvxpy/reductions/solvers/nlp_solvers/nlp_solver.py

@@ -84,45 +84,59 @@ def get_constraint_bounds(self):
         as well as equalities to zero constraints and forms
         a new problem from the canonicalized problem.
         """
-        lower, upper = [], []
+        # Preallocate arrays for constraint bounds
+        total_constr_size = sum(c.size for c in self.problem.constraints)
+        lower = np.zeros(total_constr_size)
+        upper = np.zeros(total_constr_size)
+
         new_constr = []
+        offset = 0
         for constraint in self.problem.constraints:
+            size = constraint.size
             if isinstance(constraint, Equality):
-                lower.extend([0.0] * constraint.size)
-                upper.extend([0.0] * constraint.size)
+                # lower[offset:offset+size] already 0.0
+                # upper[offset:offset+size] already 0.0
                 new_constr.append(lower_equality(constraint))
             elif isinstance(constraint, Inequality):
-                lower.extend([0.0] * constraint.size)
-                upper.extend([np.inf] * constraint.size)
+                # lower[offset:offset+size] already 0.0
+                upper[offset:offset + size] = np.inf
                 new_constr.append(lower_ineq_to_nonneg(constraint))
             elif isinstance(constraint, NonPos):
-                lower.extend([0.0] * constraint.size)
-                upper.extend([np.inf] * constraint.size)
+                # lower[offset:offset+size] already 0.0
+                upper[offset:offset + size] = np.inf
                 new_constr.append(nonpos2nonneg(constraint))
+            offset += size
+
         canonicalized_prob = self.problem.copy([self.problem.objective, new_constr])
         self.new_problem = canonicalized_prob
-        self.cl = np.array(lower)
-        self.cu = np.array(upper)
+        self.cl = lower
+        self.cu = upper
 
     def get_variable_bounds(self):
         """
         Get variable bounds for all variables.
         Uses the variable's get_bounds() method which handles bounds attributes,
         nonneg/nonpos attributes, and properly broadcasts scalar bounds.
         """
-        var_lower, var_upper = [], []
+        # Preallocate arrays for variable bounds
+        total_var_size = sum(v.size for v in self.main_var)
+        var_lower = np.full(total_var_size, -np.inf)
+        var_upper = np.full(total_var_size, np.inf)
+
+        offset = 0
         for var in self.main_var:
+            size = var.size
             # get_bounds() returns arrays broadcastable to var.shape
             # and handles all edge cases (scalar bounds, sign attributes, etc.)
             lb, ub = var.get_bounds()
             # Flatten in column-major (Fortran) order and convert to contiguous array
             # (broadcast_to creates read-only views that need to be copied)
-            lb_flat = np.asarray(lb).flatten(order='F')
-            ub_flat = np.asarray(ub).flatten(order='F')
-            var_lower.extend(lb_flat)
-            var_upper.extend(ub_flat)
-        self.lb = np.array(var_lower)
-        self.ub = np.array(var_upper)
+            var_lower[offset:offset + size] = np.asarray(lb).flatten(order='F')
+            var_upper[offset:offset + size] = np.asarray(ub).flatten(order='F')
+            offset += size
+
+        self.lb = var_lower
+        self.ub = var_upper
 
 
     def construct_initial_point(self):
@@ -213,7 +227,7 @@ def hessian(self, x, duals, obj_factor):
         """Returns the lower triangular Hessian values in COO format. """
         if not self.use_hessian:
             # Shouldn't be called when using quasi-Newton, but return empty array
-            return np.array([])
+            return np.array([], dtype=np.float64)
 
         if not self.objective_forward_passed:
             self.objective(x)
@@ -230,13 +244,17 @@ def hessian(self, x, duals, obj_factor):
 
     def hessianstructure(self):
         """Returns the sparsity structure of the lower triangular Hessian."""
-        if not self.use_hessian:
-            # Return empty structure when using quasi-Newton approximation
-            return (np.array([], dtype=np.int32), np.array([], dtype=np.int32))
-
         if self._hess_structure is not None:
             return self._hess_structure
 
+        if not self.use_hessian:
+            # Cache and return empty structure when using quasi-Newton approximation
+            self._hess_structure = (
+                np.array([], dtype=np.int32),
+                np.array([], dtype=np.int32)
+            )
+            return self._hess_structure
+
         hess_csr = self.c_problem.get_hessian()
         hess_coo = hess_csr.tocoo()