Skip to content

Commit 7daccf6

Browse files
Transurgeonclaude
andcommitted
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>
1 parent 944eb91 commit 7daccf6

4 files changed

Lines changed: 114 additions & 38 deletions

File tree

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: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,18 @@
3131

3232

3333
def convert_matmul(expr, children, var_dict, n_vars, param_dict):
34-
"""Convert matrix multiplication A @ f(x), f(x) @ A, or X @ Y."""
34+
"""Convert matrix multiplication A @ f(x), f(x) @ A, or X @ Y.
35+
36+
NumPy matmul semantics for 1D arrays:
37+
(n,) @ (m,k) → treat left as (1,n)
38+
(m,k) @ (n,) → treat right as (n,1)
39+
(n,) @ (n,) → dot product, treat as (1,n) @ (n,1) → scalar
40+
41+
The C engine only has 2D nodes. 1D expressions are stored as (1,n) by
42+
normalize_shape. For left-matmul A @ child, if the child is 1D we must
43+
reshape it to (n,1) so the inner dimensions agree. For right-matmul
44+
child @ A, the helpers already handle 1D A via the ndim==1 branch.
45+
"""
3546
left_arg, right_arg = expr.args
3647

3748
if left_arg.is_constant():
@@ -40,9 +51,15 @@ def convert_matmul(expr, children, var_dict, n_vars, param_dict):
4051
param_node = param_dict[left_arg.id]
4152
else:
4253
param_node = None
54+
child = children[1]
55+
# If the right operand is 1D, the C node is (1,n) but left-matmul
56+
# needs it as (n,1) so inner dims match: A(m,n) @ child(n,1).
57+
if len(right_arg.shape) <= 1 and right_arg.size > 1:
58+
n = right_arg.size
59+
child = _diffengine.make_reshape(child, n, 1)
4360
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)
61+
return make_sparse_left_matmul(param_node, child, A)
62+
return make_dense_left_matmul(param_node, child, A)
4663

4764
elif right_arg.is_constant():
4865
A = right_arg.value
@@ -98,11 +115,22 @@ def convert_expr(expr, var_dict, n_vars, param_dict=None):
98115
return param_dict[expr.id]
99116

100117
# Base case: constant (in the diff engine, a constant is a parameter with ID -1)
118+
# Also handles atoms applied to pure constants (e.g. PnormApprox(Constant), floor(Constant))
119+
# that weren't folded by Dcp2Cone.
101120
if isinstance(expr, cp.Constant):
102121
c = to_dense_float(expr.value)
103122
d1, d2 = normalize_shape(expr.shape)
104123
return _diffengine.make_parameter(d1, d2, -1, n_vars, c.flatten(order='F'))
105124

125+
# Constant atom: no variables/parameters, so it must have a concrete value.
126+
# Handles atoms like floor(NegExpression(Constant(5.0))) after EvalParams.
127+
# Check variables()/parameters() before .value to avoid triggering
128+
# numeric() on atoms that don't support it (e.g. cached SymbolicQuadForm).
129+
if not expr.variables() and not expr.parameters() and expr.value is not None:
130+
c = to_dense_float(expr.value)
131+
d1, d2 = normalize_shape(expr.shape)
132+
return _diffengine.make_parameter(d1, d2, -1, n_vars, c.flatten(order='F'))
133+
106134
# Recursive case: atoms
107135
atom_name = type(expr).__name__
108136
children = [convert_expr(arg, var_dict, n_vars, param_dict) for arg in expr.args]
@@ -113,6 +141,11 @@ def convert_expr(expr, var_dict, n_vars, param_dict=None):
113141
C_expr = convert_matmul(expr, children, var_dict, n_vars, param_dict)
114142
elif atom_name == "multiply":
115143
C_expr = convert_multiply(expr, children, var_dict, n_vars, param_dict)
144+
elif atom_name in ("QuadForm", "SymbolicQuadForm"):
145+
from cvxpy.reductions.solvers.nlp_solvers.diff_engine.registry import (
146+
convert_quad_form,
147+
)
148+
C_expr = convert_quad_form(expr, children, n_vars)
116149
elif atom_name in ATOM_CONVERTERS:
117150
C_expr = ATOM_CONVERTERS[atom_name](expr, children)
118151
else:
@@ -123,10 +156,16 @@ def convert_expr(expr, var_dict, n_vars, param_dict=None):
123156
d1_Python, d2_Python = normalize_shape(expr.shape)
124157

125158
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-
159+
# 1D Python shapes (n,) normalize to (1, n), but the C engine may
160+
# produce (n, 1) — e.g. matrix @ scalar or transpose of a vector.
161+
# Both represent the same 1D data; reshape to match Python convention.
162+
if len(expr.shape) <= 1 and d1_C * d2_C == d1_Python * d2_Python:
163+
C_expr = _diffengine.make_reshape(C_expr, d1_Python, d2_Python)
164+
else:
165+
raise ValueError(
166+
f"Dimension mismatch for atom '{atom_name}': "
167+
f"C dimensions ({d1_C}, {d2_C}) vs "
168+
f"Python dimensions ({d1_Python}, {d2_Python})"
169+
)
131170

132171
return C_expr

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@ 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+
if A.ndim == 1:
57+
m, n = 1, A.shape[0]
58+
else:
59+
m, n = A.shape
5760
return _diffengine.make_left_matmul(
5861
param_node, child, 'dense', A.flatten(order='C'), m, n)
5962

@@ -70,7 +73,10 @@ def make_sparse_right_matmul(param_node, child, A):
7073

7174

7275
def make_dense_right_matmul(param_node, child, A):
73-
m, n = normalize_shape(A.shape)
76+
if A.ndim == 1:
77+
m, n = A.shape[0], 1
78+
else:
79+
m, n = A.shape
7480
return _diffengine.make_right_matmul(
7581
param_node, child, 'dense', A.flatten(order='C'), m, n)
7682

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

Lines changed: 43 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -92,42 +92,58 @@ def convert_rel_entr(expr, children):
9292
return _diffengine.make_rel_entr(children[0], children[1])
9393

9494

95-
def convert_quad_form(expr, children):
96-
"""Convert quadratic form x.T @ P @ x."""
95+
def convert_quad_form(expr, children, n_vars):
96+
"""Convert quadratic form x.T @ P @ x.
9797
98+
Currently only handles scalar quad forms (shape ()). Vector-shaped
99+
SymbolicQuadForm (e.g. from huber, quad_over_lin with axis) requires
100+
native block quadform support in SparseDiffPy.
101+
"""
98102
P = expr.args[1]
99103

100-
if not isinstance(P, cp.Constant):
104+
if not P.is_constant():
101105
raise NotImplementedError("quad_form requires P to be a constant matrix")
102106

103-
P = P.value
107+
# TODO: vector-shaped SymbolicQuadForm (e.g. from huber, quad_over_lin
108+
# with axis) needs native block quadform support in SparseDiffPy rather
109+
# than decomposing into diag(P) * power(x, 2).
110+
if expr.size > 1:
111+
raise NotImplementedError(
112+
f"Vector-shaped quad form (shape {expr.shape}) not yet supported "
113+
"by the diffengine backend. Needs native block quadform in SparseDiffPy."
114+
)
104115

105-
if not isinstance(P, sparse.csr_matrix):
106-
P = sparse.csr_matrix(P)
116+
P_val = P.value
117+
if sparse.issparse(P_val):
118+
P_val = P_val.toarray()
119+
P_val = np.asarray(P_val, dtype=np.float64)
107120

121+
P_csr = sparse.csr_matrix(P_val)
108122
return _diffengine.make_quad_form(
109123
children[0],
110-
P.data.astype(np.float64),
111-
P.indices.astype(np.int32),
112-
P.indptr.astype(np.int32),
113-
P.shape[0],
114-
P.shape[1],
124+
P_csr.data.astype(np.float64),
125+
P_csr.indices.astype(np.int32),
126+
P_csr.indptr.astype(np.int32),
127+
P_csr.shape[0],
128+
P_csr.shape[1],
115129
)
116130

117131

118132
def convert_reshape(expr, children):
119-
"""Convert reshape - only Fortran order is supported.
133+
"""Convert reshape.
120134
121-
Note: Only order='F' (Fortran/column-major) is supported.
135+
F-order (column-major) uses make_reshape directly.
136+
C-order (row-major) is decomposed as: transpose(reshape(transpose(x), (n, m), F))
137+
since reshape(x, (m, n), C) == transpose(reshape(transpose(x), (n, m), F)).
122138
"""
123-
if expr.order != "F":
124-
raise NotImplementedError(
125-
f"reshape with order='{expr.order}' not supported. "
126-
"Only order='F' (Fortran) is currently supported."
127-
)
128-
129139
d1, d2 = normalize_shape(expr.shape)
130-
return _diffengine.make_reshape(children[0], d1, d2)
140+
if expr.order == "F":
141+
return _diffengine.make_reshape(children[0], d1, d2)
142+
else:
143+
# C-order: transpose input, F-reshape to swapped dims, transpose output
144+
transposed = _diffengine.make_transpose(children[0])
145+
reshaped = _diffengine.make_reshape(transposed, d2, d1)
146+
return _diffengine.make_transpose(reshaped)
131147

132148
def convert_broadcast(expr, children):
133149
d1, d2 = expr.broadcast_shape
@@ -173,9 +189,12 @@ def convert_prod(expr, children):
173189
return _diffengine.make_prod_axis_one(children[0])
174190

175191
def convert_transpose(expr, children):
176-
# If the child is a vector (shape (n,) or (n,1) or (1,n)), use reshape to transpose
177-
child_shape = normalize_shape(expr.args[0].shape)
192+
# In CVXPY, transposing a 1D vector (n,) is a no-op: (n,).T == (n,).
193+
# The C engine stores 1D as (1, n), so we must not flip it to (n, 1).
194+
if len(expr.args[0].shape) <= 1:
195+
return children[0]
178196

197+
child_shape = normalize_shape(expr.args[0].shape)
179198
if 1 in child_shape:
180199
return _diffengine.make_reshape(children[0], child_shape[1], child_shape[0])
181200
else:
@@ -229,8 +248,8 @@ def convert_upper_tri(_expr, children):
229248
# Reductions
230249
"Sum": convert_sum,
231250
# Bivariate
232-
"QuadForm": convert_quad_form,
233-
"SymbolicQuadForm": convert_quad_form,
251+
# QuadForm and SymbolicQuadForm are dispatched directly in convert_expr
252+
# (they need n_vars for creating constant nodes in the vector case).
234253
"quad_over_lin": convert_quad_over_lin,
235254
"rel_entr": convert_rel_entr,
236255
# Elementwise univariate with parameter

0 commit comments

Comments
 (0)