From be3270d0937600c26b72af164c61b4a1c97044b3 Mon Sep 17 00:00:00 2001 From: William Zijie Zhang Date: Tue, 24 Feb 2026 11:51:32 -0800 Subject: [PATCH 1/2] Address review comments: remove redundant nonsmooth methods, add types, docs - Remove is_atom_nonsmooth_convex/is_atom_nonsmooth_concave from base Atom class and 10 atom subclasses; use existing is_atom_convex/is_atom_concave in DNLP composition rules instead - Add type annotations and docstrings to NLPsolver, Bounds, and Oracles in nlp_solver.py - Document Variable.sample_bounds with class-level type annotation and docstring - Revert GENERAL_PROJECTION_TOL back to 1e-10 (was loosened for removed IPOPT derivative checker) - Update CLAUDE.md atom classification docs to reflect simplified API Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 12 +-- cvxpy/atoms/atom.py | 14 +-- cvxpy/atoms/elementwise/abs.py | 4 - cvxpy/atoms/elementwise/huber.py | 4 - cvxpy/atoms/elementwise/maximum.py | 4 - cvxpy/atoms/elementwise/minimum.py | 4 - cvxpy/atoms/max.py | 4 - cvxpy/atoms/min.py | 4 - cvxpy/atoms/norm1.py | 4 - cvxpy/atoms/norm_inf.py | 4 - cvxpy/atoms/pnorm.py | 8 -- cvxpy/atoms/sum_largest.py | 4 - cvxpy/expressions/variable.py | 10 +- .../solvers/nlp_solvers/nlp_solver.py | 99 +++++++++++++------ cvxpy/settings.py | 2 +- 15 files changed, 90 insertions(+), 91 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 49ab45af2c..23c1383c1e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -159,7 +159,7 @@ Adding a new diff engine atom requires: The diff engine supports CVXPY `Parameter` objects: `C_problem` registers parameters with the C engine and `update_params()` re-pushes values without rebuilding the expression tree. Sparse parameter values are fused into sparse matmul operations. -`DerivativeChecker` in `nlp_solver.py` provides finite-difference verification of gradients, Jacobians, and Hessians during development. +`DerivativeChecker` in `cvxpy/tests/nlp_tests/derivative_checker.py` provides finite-difference verification of gradients, Jacobians, and Hessians during development. Key files in `cvxpy/reductions/solvers/nlp_solvers/diff_engine/`: - `converters.py` - Converts CVXPY expression AST to C diff engine trees. Contains `ATOM_CONVERTERS` dict mapping ~40 atom types to C constructors. Includes optimizations like sparse parameter matmul fusion. @@ -182,17 +182,17 @@ Convention: all arrays are flattened in **Fortran order** ('F') for column-major 1. Create a canonicalizer in `cvxpy/reductions/dnlp2smooth/canonicalizers/` 2. The canonicalizer converts non-smooth atoms to smooth equivalents using auxiliary variables 3. Register in `canonicalizers/__init__.py` by adding to `SMOOTH_CANON_METHODS` dict -4. Classify the atom using the three-way atom-level API (see below) +4. If the atom is smooth, override `is_atom_smooth()` to return `True` (see classification below) -### DNLP Atom Classification (Three-way) +### DNLP Atom Classification -Each atom implements exactly one of three atom-level methods: +Atoms are classified as smooth or non-smooth. Non-smooth atoms reuse the existing `is_atom_convex()`/`is_atom_concave()` methods for the DNLP composition rules—no separate method is needed. | Category | Method | Examples | |---|---|---| | **Smooth** | `is_atom_smooth() → True` | exp, log, power, sin, prod, quad_form | -| **NS-convex** | `is_atom_nonsmooth_convex() → True` | abs, max, norm1, norm_inf, huber | -| **NS-concave** | `is_atom_nonsmooth_concave() → True` | min, minimum | +| **Non-smooth convex** | `is_atom_convex() → True` (and not smooth) | abs, max, norm1, norm_inf, huber | +| **Non-smooth concave** | `is_atom_concave() → True` (and not smooth) | min, minimum | ### DNLP Expression-level Rules diff --git a/cvxpy/atoms/atom.py b/cvxpy/atoms/atom.py index 6b1dff3daf..a9918db541 100644 --- a/cvxpy/atoms/atom.py +++ b/cvxpy/atoms/atom.py @@ -192,14 +192,6 @@ def is_atom_smooth(self) -> bool: """Is the atom smooth?""" return False - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return False - - def is_atom_nonsmooth_concave(self) -> bool: - """Is the atom nonsmooth and concave?""" - return False - def is_atom_log_log_convex(self) -> bool: """Is the atom log-log convex? """ @@ -278,7 +270,7 @@ def is_linearizable_convex(self) -> bool: # Applies DNLP composition rule. if self.is_constant(): return True - elif self.is_atom_smooth() or self.is_atom_nonsmooth_convex(): + elif self.is_atom_smooth() or self.is_atom_convex(): for idx, arg in enumerate(self.args): if not (arg.is_smooth() or (arg.is_linearizable_convex() and self.is_incr(idx)) or @@ -287,7 +279,7 @@ def is_linearizable_convex(self) -> bool: return True else: return False - + @perf.compute_once def is_linearizable_concave(self) -> bool: """Is the expression concave after linearizing all smooth subexpressions? @@ -295,7 +287,7 @@ def is_linearizable_concave(self) -> bool: # Applies DNLP composition rule. if self.is_constant(): return True - elif self.is_atom_smooth() or self.is_atom_nonsmooth_concave(): + elif self.is_atom_smooth() or self.is_atom_concave(): for idx, arg in enumerate(self.args): if not (arg.is_smooth() or (arg.is_linearizable_concave() and self.is_incr(idx)) or diff --git a/cvxpy/atoms/elementwise/abs.py b/cvxpy/atoms/elementwise/abs.py index 18b1b10f8f..5fe0bd17f5 100644 --- a/cvxpy/atoms/elementwise/abs.py +++ b/cvxpy/atoms/elementwise/abs.py @@ -55,10 +55,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return False - - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return True def is_incr(self, idx) -> bool: """Is the composition non-decreasing in argument idx? diff --git a/cvxpy/atoms/elementwise/huber.py b/cvxpy/atoms/elementwise/huber.py index cb7ee5c336..fa7caaaf27 100644 --- a/cvxpy/atoms/elementwise/huber.py +++ b/cvxpy/atoms/elementwise/huber.py @@ -70,10 +70,6 @@ def is_atom_convex(self) -> bool: def is_atom_concave(self) -> bool: """Is the atom concave?""" return False - - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return True def is_incr(self, idx) -> bool: """Is the composition non-decreasing in argument idx?""" diff --git a/cvxpy/atoms/elementwise/maximum.py b/cvxpy/atoms/elementwise/maximum.py index dc8df04589..2dfa058c82 100644 --- a/cvxpy/atoms/elementwise/maximum.py +++ b/cvxpy/atoms/elementwise/maximum.py @@ -66,10 +66,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return False - - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return True def is_atom_log_log_convex(self) -> bool: """Is the atom log-log convex? diff --git a/cvxpy/atoms/elementwise/minimum.py b/cvxpy/atoms/elementwise/minimum.py index fbbc5ae8e3..1bd0386fdd 100644 --- a/cvxpy/atoms/elementwise/minimum.py +++ b/cvxpy/atoms/elementwise/minimum.py @@ -58,10 +58,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return True - - def is_atom_nonsmooth_concave(self) -> bool: - """Is the atom nonsmooth and concave?""" - return True def is_atom_log_log_convex(self) -> bool: """Is the atom log-log convex? diff --git a/cvxpy/atoms/max.py b/cvxpy/atoms/max.py index 2824f47f1b..a305dc5444 100644 --- a/cvxpy/atoms/max.py +++ b/cvxpy/atoms/max.py @@ -99,10 +99,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return False - - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return True def is_atom_log_log_convex(self) -> bool: """Is the atom log-log convex? diff --git a/cvxpy/atoms/min.py b/cvxpy/atoms/min.py index 8d2c98d8e1..df32d919db 100644 --- a/cvxpy/atoms/min.py +++ b/cvxpy/atoms/min.py @@ -99,10 +99,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return True - - def is_atom_nonsmooth_concave(self) -> bool: - """Is the atom nonsmooth and concave?""" - return True def is_atom_log_log_convex(self) -> bool: """Is the atom log-log convex? diff --git a/cvxpy/atoms/norm1.py b/cvxpy/atoms/norm1.py index 0ab4b75ac9..d88823c180 100644 --- a/cvxpy/atoms/norm1.py +++ b/cvxpy/atoms/norm1.py @@ -55,10 +55,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return False - - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return True def is_incr(self, idx) -> bool: """Is the composition non-decreasing in argument idx? diff --git a/cvxpy/atoms/norm_inf.py b/cvxpy/atoms/norm_inf.py index 4bbc96a8cf..c8f8221d3e 100644 --- a/cvxpy/atoms/norm_inf.py +++ b/cvxpy/atoms/norm_inf.py @@ -58,10 +58,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return False - - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return True def is_atom_log_log_convex(self) -> bool: """Is the atom log-log convex? diff --git a/cvxpy/atoms/pnorm.py b/cvxpy/atoms/pnorm.py index e0ecbf52cc..6dac190947 100644 --- a/cvxpy/atoms/pnorm.py +++ b/cvxpy/atoms/pnorm.py @@ -176,14 +176,6 @@ def is_atom_concave(self) -> bool: """ return self.p < 1 - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return self.p >= 1 - - def is_atom_nonsmooth_concave(self) -> bool: - """Is the atom nonsmooth and concave?""" - return self.p < 1 - def is_atom_log_log_convex(self) -> bool: """Is the atom log-log convex? """ diff --git a/cvxpy/atoms/sum_largest.py b/cvxpy/atoms/sum_largest.py index d237748f6c..23ae364b79 100644 --- a/cvxpy/atoms/sum_largest.py +++ b/cvxpy/atoms/sum_largest.py @@ -118,10 +118,6 @@ def is_atom_concave(self) -> bool: """Is the atom concave? """ return False - - def is_atom_nonsmooth_convex(self) -> bool: - """Is the atom nonsmooth and convex?""" - return True def is_incr(self, idx) -> bool: """Is the composition non-decreasing in argument idx? diff --git a/cvxpy/expressions/variable.py b/cvxpy/expressions/variable.py index 3c5166049f..a2d6b1fb10 100644 --- a/cvxpy/expressions/variable.py +++ b/cvxpy/expressions/variable.py @@ -18,6 +18,8 @@ from typing import TYPE_CHECKING, Any, Iterable, Optional, Tuple if TYPE_CHECKING: + import numpy as np + from cvxpy.expressions.constants.parameter import Parameter import scipy.sparse as sp @@ -33,6 +35,13 @@ class Variable(Leaf): """The optimization variables in a problem.""" + #: Explicit bounds for random initial point sampling in ``best_of`` NLP solves. + #: A tuple ``(low, high)`` of array_like values broadcastable to the variable shape, + #: or ``None``. When set, overrides the variable's ``value`` during random + #: initialization. When ``None`` and finite ``bounds`` are present, those are + #: used instead. See :meth:`Problem.set_random_NLP_initial_point`. + sample_bounds: tuple[np.ndarray, np.ndarray] | None + def __init__( self, shape: int | Iterable[int] = (), name: str | None = None, var_id: int | None = None, **kwargs: Any @@ -51,7 +60,6 @@ def __init__( self._value = None self.delta = None self.gradient = None - # bounds for sampling initial points in DNLP problems self.sample_bounds = None super(Variable, self).__init__(shape, **kwargs) diff --git a/cvxpy/reductions/solvers/nlp_solvers/nlp_solver.py b/cvxpy/reductions/solvers/nlp_solvers/nlp_solver.py index 6e3f8a4f80..178069aec2 100644 --- a/cvxpy/reductions/solvers/nlp_solvers/nlp_solver.py +++ b/cvxpy/reductions/solvers/nlp_solvers/nlp_solver.py @@ -14,6 +14,10 @@ limitations under the License. """ +from __future__ import annotations + +from typing import TYPE_CHECKING + import numpy as np from cvxpy.constraints import ( @@ -29,6 +33,9 @@ nonpos2nonneg, ) +if TYPE_CHECKING: + from cvxpy.problems.problem import Problem + class NLPsolver(Solver): """ @@ -37,13 +44,13 @@ class NLPsolver(Solver): REQUIRES_CONSTR = False MIP_CAPABLE = False - def accepts(self, problem): + def accepts(self, problem: Problem) -> bool: """ Only accepts disciplined nonlinear programs. """ return problem.is_dnlp() - def apply(self, problem): + def apply(self, problem: Problem) -> tuple[dict, InverseData]: """ Construct NLP problem data stored in a dictionary. The NLP has the following form @@ -57,7 +64,9 @@ def apply(self, problem): return data, inv_data - def _prepare_data_and_inv_data(self, problem): + def _prepare_data_and_inv_data( + self, problem: Problem + ) -> tuple[Problem, dict, InverseData]: data = dict() bounds = Bounds(problem) inverse_data = InverseData(bounds.new_problem) @@ -69,15 +78,26 @@ def _prepare_data_and_inv_data(self, problem): data["_bounds"] = bounds # Store for deferred Oracles creation in solve_via_data return problem, data, inverse_data -class Bounds(): - def __init__(self, problem): +class Bounds: + """Extracts variable and constraint bounds from a CVXPY problem. + + Converts the problem into the standard NLP form:: + + g^l <= g(x) <= g^u, x^l <= x <= x^u + + Inequalities are lowered to nonneg form and equalities to zero + constraints. The resulting ``new_problem`` attribute holds the + canonicalized problem used by the solver oracles. + """ + + def __init__(self, problem: Problem) -> None: self.problem = problem self.main_var = problem.variables() self.get_constraint_bounds() self.get_variable_bounds() self.construct_initial_point() - def get_constraint_bounds(self): + def get_constraint_bounds(self) -> None: """ Get constraint bounds for all constraints. Also converts inequalities to nonneg form, @@ -104,7 +124,7 @@ def get_constraint_bounds(self): self.cl = np.array(lower) self.cu = np.array(upper) - def get_variable_bounds(self): + def get_variable_bounds(self) -> None: """ Get variable bounds for all variables. Uses the variable's get_bounds() method which handles bounds attributes, @@ -123,9 +143,8 @@ def get_variable_bounds(self): var_upper.extend(ub_flat) self.lb = np.array(var_lower) self.ub = np.array(var_upper) - - def construct_initial_point(self): + def construct_initial_point(self) -> None: """ Loop through all variables and collect the intial point.""" x0 = [] for var in self.main_var: @@ -136,16 +155,29 @@ def construct_initial_point(self): x0.append(np.atleast_1d(var.value).flatten(order='F')) self.x0 = np.concatenate(x0, axis=0) -class Oracles(): - """ - Oracle interface for NLP solvers using the C-based diff engine. +class Oracles: + """Oracle interface for NLP solvers using the C-based diff engine. Provides function and derivative oracles (objective, gradient, constraints, - jacobian, hessian) by wrapping the C_problem class from dnlp_diff_engine. + Jacobian, Hessian) by wrapping the ``C_problem`` class from the diff engine. + + Forward passes are cached per solver iteration: calling ``objective`` or + ``constraints`` sets a flag so that ``gradient``/``jacobian``/``hessian`` + can reuse the cached forward values. The ``intermediate`` callback resets + these flags at the start of each new solver iteration. + + Sparsity structures (Jacobian and Hessian) are computed once on first + access and cached for the lifetime of the object. """ - def __init__(self, problem, initial_point, num_constraints, - verbose: bool = True, use_hessian: bool = True): + def __init__( + self, + problem: Problem, + initial_point: np.ndarray, + num_constraints: int, + verbose: bool = True, + use_hessian: bool = True, + ) -> None: # Import from cvxpy's diff_engine integration layer from cvxpy.reductions.solvers.nlp_solvers.diff_engine import C_problem @@ -164,30 +196,30 @@ def __init__(self, problem, initial_point, num_constraints, self.iterations = 0 # Cached sparsity structures - self._jac_structure = None - self._hess_structure = None + self._jac_structure: tuple[np.ndarray, np.ndarray] | None = None + self._hess_structure: tuple[np.ndarray, np.ndarray] | None = None self.constraints_forward_passed = False self.objective_forward_passed = False - def objective(self, x): + def objective(self, x: np.ndarray) -> float: """Returns the scalar value of the objective given x.""" self.objective_forward_passed = True return self.c_problem.objective_forward(x) - def gradient(self, x): + def gradient(self, x: np.ndarray) -> np.ndarray: """Returns the gradient of the objective with respect to x.""" - + if not self.objective_forward_passed: self.objective(x) return self.c_problem.gradient() - def constraints(self, x): + def constraints(self, x: np.ndarray) -> np.ndarray: """Returns the constraint values.""" self.constraints_forward_passed = True return self.c_problem.constraint_forward(x) - def jacobian(self, x): + def jacobian(self, x: np.ndarray) -> np.ndarray: """Returns the Jacobian values in COO format at the sparsity structure. """ if not self.constraints_forward_passed: @@ -197,7 +229,7 @@ def jacobian(self, x): jac_coo = jac_csr.tocoo() return jac_coo.data.copy() - def jacobianstructure(self): + def jacobianstructure(self) -> tuple[np.ndarray, np.ndarray]: """Returns the sparsity structure of the Jacobian.""" if self._jac_structure is not None: return self._jac_structure @@ -209,7 +241,7 @@ def jacobianstructure(self): jac_coo.col.astype(np.int32)) return self._jac_structure - def hessian(self, x, duals, obj_factor): + def hessian(self, x: np.ndarray, duals: np.ndarray, obj_factor: float) -> np.ndarray: """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 @@ -228,7 +260,7 @@ def hessian(self, x, duals, obj_factor): return hess_coo.data[mask] - def hessianstructure(self): + def hessianstructure(self) -> tuple[np.ndarray, np.ndarray]: """Returns the sparsity structure of the lower triangular Hessian.""" if not self.use_hessian: # Return empty structure when using quasi-Newton approximation @@ -248,9 +280,20 @@ def hessianstructure(self): ) return self._hess_structure - def intermediate(self, alg_mod, iter_count, obj_value, inf_pr, inf_du, mu, - d_norm, regularization_size, alpha_du, alpha_pr, - ls_trials): + def intermediate( + self, + alg_mod: int, + iter_count: int, + obj_value: float, + inf_pr: float, + inf_du: float, + mu: float, + d_norm: float, + regularization_size: float, + alpha_du: float, + alpha_pr: float, + ls_trials: int, + ) -> None: """Prints information at every Ipopt iteration.""" self.iterations = iter_count self.objective_forward_passed = False diff --git a/cvxpy/settings.py b/cvxpy/settings.py index 7e36167ef4..45b3269296 100644 --- a/cvxpy/settings.py +++ b/cvxpy/settings.py @@ -205,7 +205,7 @@ # Numerical tolerances EIGVAL_TOL = 1e-10 PSD_NSD_PROJECTION_TOL = 1e-8 -GENERAL_PROJECTION_TOL = 1e-6 +GENERAL_PROJECTION_TOL = 1e-10 SPARSE_PROJECTION_TOL = 1e-10 ATOM_EVAL_TOL = 1e-4 CHOL_SYM_TOL = 1e-14 From 3bf59e7c662e6eb9aa74451d2600c70358ef7c63 Mon Sep 17 00:00:00 2001 From: William Zijie Zhang Date: Tue, 24 Feb 2026 11:59:55 -0800 Subject: [PATCH 2/2] Move sample_bounds docs from #: comments into Variable class docstring Co-Authored-By: Claude Opus 4.6 --- cvxpy/expressions/variable.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cvxpy/expressions/variable.py b/cvxpy/expressions/variable.py index a2d6b1fb10..b55da0f8eb 100644 --- a/cvxpy/expressions/variable.py +++ b/cvxpy/expressions/variable.py @@ -18,8 +18,6 @@ from typing import TYPE_CHECKING, Any, Iterable, Optional, Tuple if TYPE_CHECKING: - import numpy as np - from cvxpy.expressions.constants.parameter import Parameter import scipy.sparse as sp @@ -33,14 +31,16 @@ class Variable(Leaf): - """The optimization variables in a problem.""" - - #: Explicit bounds for random initial point sampling in ``best_of`` NLP solves. - #: A tuple ``(low, high)`` of array_like values broadcastable to the variable shape, - #: or ``None``. When set, overrides the variable's ``value`` during random - #: initialization. When ``None`` and finite ``bounds`` are present, those are - #: used instead. See :meth:`Problem.set_random_NLP_initial_point`. - sample_bounds: tuple[np.ndarray, np.ndarray] | None + """The optimization variables in a problem. + + Attributes + ---------- + sample_bounds : tuple[np.ndarray, np.ndarray] | None + Explicit bounds ``(low, high)`` for random initial point sampling in + ``best_of`` NLP solves. When set, overrides the variable's ``value`` + during random initialization. When ``None`` and finite ``bounds`` are + present, those are used instead. + """ def __init__( self, shape: int | Iterable[int] = (), name: str | None = None,