Skip to content

Commit b8d716a

Browse files
Transurgeonclaude
andauthored
fix diff engine converter bugs: 1D matmul dims, reshape, transpose, q… (#190)
* fix diff engine converter bugs: 1D matmul dims, reshape, transpose, quad form - helpers.py: fix 1D dimension normalization in dense matmul helpers. Right-matmul treated 1D A as (1,n) instead of (n,1), causing segfaults. - converters.py: reshape 1D child to column vector in left-matmul for dot products; add constant-atom fallback for unfolded expressions; move QuadForm/SymbolicQuadForm dispatch out of ATOM_CONVERTERS (needs n_vars); tolerate 1D dimension mismatches via reshape instead of error. - registry.py: support C-order reshape via transpose decomposition; fix 1D transpose to be a no-op; improve scalar quad form P handling; raise NotImplementedError for vector SymbolicQuadForm (TODO: native block quadform in SparseDiffPy). - perspective_canon.py: handle DiffengineConeProgram which stores q, d, A, b as separate arrays (vs ParamConeProg sparse tensors). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * adds converters changes for right matmul support * fixes crashes for left matmul * add changes to registry * run linter --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 944eb91 commit b8d716a

6 files changed

Lines changed: 280 additions & 45 deletions

File tree

cvxpy/cvxcore/python/canonInterface.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,13 @@ def get_problem_matrix(linOps,
292292
default_canon_backend = get_default_canon_backend()
293293
canon_backend = default_canon_backend if not canon_backend else canon_backend
294294

295+
# DIFFENGINE is a reduction-layer replacement for ConeMatrixStuffing and
296+
# has no lin_ops backend of its own. When code paths reach this matrix
297+
# builder directly (e.g. tests that bypass the stuffing reduction), fall
298+
# through to the CPP backend so the lin_ops pipeline still works.
299+
if canon_backend == s.DIFFENGINE_BACKEND:
300+
canon_backend = s.CPP_CANON_BACKEND
301+
295302
if canon_backend == s.CPP_CANON_BACKEND:
296303
from cvxpy.cvxcore.python.cppbackend import build_matrix
297304
return build_matrix(id_to_col, param_to_size, param_to_col, var_length, constr_length, linOps)

cvxpy/reductions/dcp2cone/canonicalizers/perspective_canon.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,22 @@ def perspective_canon(expr, args, solver_context: SolverInfo | None = None):
3939
prob_canon = chain.apply(aux_prob)[0] # grab problem instance
4040
# get cone representation of c, A, and b for some problem.
4141

42-
q = prob_canon.q.toarray().flatten()[:-1]
43-
d = prob_canon.q.toarray().flatten()[-1]
44-
Ab = prob_canon.A.toarray().reshape((-1, len(q) + 1), order="F")
45-
A, b = Ab[:, :-1], Ab[:, -1]
42+
# Extract cone representation q, d, A, b.
43+
# ParamConeProg stores q as sparse (n+1, n_params+1) tensor with d
44+
# embedded in the last row, and A as a flattened tensor with b embedded.
45+
# DiffengineConeProgram stores q, d, A, b as separate concrete arrays.
46+
if hasattr(prob_canon, 'd') and not hasattr(prob_canon.q, 'toarray'):
47+
# DiffengineConeProgram: concrete matrices, d stored separately.
48+
q = prob_canon.q
49+
d = prob_canon.d
50+
A = prob_canon.A.toarray() if hasattr(prob_canon.A, 'toarray') else prob_canon.A
51+
b = prob_canon.b
52+
else:
53+
# ParamConeProg: sparse tensor with embedded offsets.
54+
q = prob_canon.q.toarray().flatten()[:-1]
55+
d = prob_canon.q.toarray().flatten()[-1]
56+
Ab = prob_canon.A.toarray().reshape((-1, len(q) + 1), order="F")
57+
A, b = Ab[:, :-1], Ab[:, -1]
4658

4759
# given f in epigraph form, aka epi f = \{(x,t) | f(x) \leq t\}
4860
# = \{(x,t) | Fx +tg + e \in K} for K a cone, the epigraph of the

cvxpy/reductions/solvers/nlp_solvers/diff_engine/converters.py

Lines changed: 68 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,32 +30,65 @@
3030
from cvxpy.reductions.solvers.nlp_solvers.diff_engine.registry import ATOM_CONVERTERS
3131

3232

33+
def _matmul_normalize_1d(A, side):
34+
"""Reshape a 1D numpy array to 2D for matmul.
35+
36+
NumPy matmul treats 1D arrays differently depending on which side:
37+
Left 1D: (k,) → (1, k) — row vector
38+
Right 1D: (k,) → (k, 1) — column vector
39+
2D input is returned unchanged.
40+
"""
41+
if A.ndim == 1:
42+
return A.reshape(1, -1) if side == 'left' else A.reshape(-1, 1)
43+
return A
44+
45+
3346
def convert_matmul(expr, children, var_dict, n_vars, param_dict):
34-
"""Convert matrix multiplication A @ f(x), f(x) @ A, or X @ Y."""
47+
"""Convert matrix multiplication A @ f(x), f(x) @ A, or X @ Y.
48+
49+
NumPy matmul semantics for 1D arrays:
50+
(n,) @ (m,k) → treat left as (1,n) — normalize_shape already does this
51+
(m,k) @ (n,) → treat right as (n,1) — must reshape from (1,n) storage
52+
(n,) @ (n,) → dot product: (1,n) @ (n,1) → scalar
53+
54+
The C engine only has 2D nodes. 1D expressions are stored as (1,n) by
55+
normalize_shape. All 1D→2D matmul normalization is handled here so that
56+
helper functions always receive properly shaped 2D data.
57+
"""
3558
left_arg, right_arg = expr.args
59+
left_child, right_child = children
60+
61+
# Right 1D child: C stores as (1, n) but matmul needs (n, 1).
62+
# Do this once, before branching — used by all three branches.
63+
if len(right_arg.shape) <= 1 and right_arg.size > 1:
64+
right_child = _diffengine.make_reshape(right_child, right_arg.size, 1)
3665

3766
if left_arg.is_constant():
38-
A = left_arg.value
67+
A = _matmul_normalize_1d(left_arg.value, 'left')
3968
if isinstance(left_arg, cp.Parameter):
4069
param_node = param_dict[left_arg.id]
70+
elif left_arg.parameters():
71+
param_node = left_child
4172
else:
4273
param_node = None
4374
if sparse.issparse(A):
44-
return make_sparse_left_matmul(param_node, children[1], A)
45-
return make_dense_left_matmul(param_node, children[1], A)
75+
return make_sparse_left_matmul(param_node, right_child, A)
76+
return make_dense_left_matmul(param_node, right_child, A)
4677

4778
elif right_arg.is_constant():
48-
A = right_arg.value
79+
A = _matmul_normalize_1d(right_arg.value, 'right')
4980
if isinstance(right_arg, cp.Parameter):
5081
param_node = param_dict[right_arg.id]
82+
elif right_arg.parameters():
83+
param_node = right_child
5184
else:
5285
param_node = None
5386
if sparse.issparse(A):
54-
return make_sparse_right_matmul(param_node, children[0], A)
55-
return make_dense_right_matmul(param_node, children[0], A)
87+
return make_sparse_right_matmul(param_node, left_child, A)
88+
return make_dense_right_matmul(param_node, left_child, A)
5689

5790
else:
58-
return _diffengine.make_matmul(children[0], children[1])
91+
return _diffengine.make_matmul(left_child, right_child)
5992

6093
# TODO we should support sparse elementwise multiply at some point.
6194
def convert_multiply(expr, children, var_dict, n_vars, param_dict):
@@ -98,11 +131,22 @@ def convert_expr(expr, var_dict, n_vars, param_dict=None):
98131
return param_dict[expr.id]
99132

100133
# Base case: constant (in the diff engine, a constant is a parameter with ID -1)
134+
# Also handles atoms applied to pure constants (e.g. PnormApprox(Constant), floor(Constant))
135+
# that weren't folded by Dcp2Cone.
101136
if isinstance(expr, cp.Constant):
102137
c = to_dense_float(expr.value)
103138
d1, d2 = normalize_shape(expr.shape)
104139
return _diffengine.make_parameter(d1, d2, -1, n_vars, c.flatten(order='F'))
105140

141+
# Constant atom: no variables/parameters, so it must have a concrete value.
142+
# Handles atoms like floor(NegExpression(Constant(5.0))) after EvalParams.
143+
# Check variables()/parameters() before .value to avoid triggering
144+
# numeric() on atoms that don't support it (e.g. cached SymbolicQuadForm).
145+
if not expr.variables() and not expr.parameters() and expr.value is not None:
146+
c = to_dense_float(expr.value)
147+
d1, d2 = normalize_shape(expr.shape)
148+
return _diffengine.make_parameter(d1, d2, -1, n_vars, c.flatten(order='F'))
149+
106150
# Recursive case: atoms
107151
atom_name = type(expr).__name__
108152
children = [convert_expr(arg, var_dict, n_vars, param_dict) for arg in expr.args]
@@ -113,6 +157,11 @@ def convert_expr(expr, var_dict, n_vars, param_dict=None):
113157
C_expr = convert_matmul(expr, children, var_dict, n_vars, param_dict)
114158
elif atom_name == "multiply":
115159
C_expr = convert_multiply(expr, children, var_dict, n_vars, param_dict)
160+
elif atom_name in ("QuadForm", "SymbolicQuadForm"):
161+
from cvxpy.reductions.solvers.nlp_solvers.diff_engine.registry import (
162+
convert_quad_form,
163+
)
164+
C_expr = convert_quad_form(expr, children, n_vars)
116165
elif atom_name in ATOM_CONVERTERS:
117166
C_expr = ATOM_CONVERTERS[atom_name](expr, children)
118167
else:
@@ -123,10 +172,16 @@ def convert_expr(expr, var_dict, n_vars, param_dict=None):
123172
d1_Python, d2_Python = normalize_shape(expr.shape)
124173

125174
if d1_C != d1_Python or d2_C != d2_Python:
126-
raise ValueError(
127-
f"Dimension mismatch for atom '{atom_name}': "
128-
f"C dimensions ({d1_C}, {d2_C}) vs Python dimensions ({d1_Python}, {d2_Python})"
129-
)
130-
175+
# 1D Python shapes (n,) normalize to (1, n), but the C engine may
176+
# produce (n, 1) — e.g. matrix @ scalar or transpose of a vector.
177+
# Both represent the same 1D data; reshape to match Python convention.
178+
if len(expr.shape) <= 1 and d1_C * d2_C == d1_Python * d2_Python:
179+
C_expr = _diffengine.make_reshape(C_expr, d1_Python, d2_Python)
180+
else:
181+
raise ValueError(
182+
f"Dimension mismatch for atom '{atom_name}': "
183+
f"C dimensions ({d1_C}, {d2_C}) vs "
184+
f"Python dimensions ({d1_Python}, {d2_Python})"
185+
)
131186

132187
return C_expr

cvxpy/reductions/solvers/nlp_solvers/diff_engine/helpers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def make_sparse_left_matmul(param_node, child, A):
5353

5454

5555
def make_dense_left_matmul(param_node, child, A):
56-
m, n = normalize_shape(A.shape)
56+
m, n = A.shape
5757
return _diffengine.make_left_matmul(
5858
param_node, child, 'dense', A.flatten(order='C'), m, n)
5959

@@ -70,7 +70,7 @@ def make_sparse_right_matmul(param_node, child, A):
7070

7171

7272
def make_dense_right_matmul(param_node, child, A):
73-
m, n = normalize_shape(A.shape)
73+
m, n = A.shape
7474
return _diffengine.make_right_matmul(
7575
param_node, child, 'dense', A.flatten(order='C'), m, n)
7676

0 commit comments

Comments
 (0)