Skip to content

Add an offset argument k to diag_vec / diag_mat #119

Description

@Transurgeon

CVXPY's cp.diag(x, k) accepts a diagonal offset, and every existing CVXPY canonicalization backend supports it via the diag_vec/diag_mat LinOp data. The diff engine only implements the main diagonal, so the CVXPY-side converter has to reject k != 0:

NotImplementedError: diag_vec with k != 0 not supported in diff engine

This is currently the last engine-side atom gap blocking DIFFENGINE from becoming CVXPY's default canonicalization backend for parameter-free problems (the other, Concatenate, is CVXPY-side). An earlier attempt emulated the offset with selector matmuls in Python; that was dropped as the wrong layer — it belongs in the engine's node constructors.

Requested API

new_diag_vec(expr *child, int k) and new_diag_mat(expr *child, int k) (include/atoms/affine.h:39-40), with k threaded through the SparseDiffPy bindings py_make_diag_vec / py_make_diag_mat, which today parse a single capsule with PyArg_ParseTuple(args, "O", ...) (sparsediffpy/_bindings/atoms/diag_vec.h, diag_mat.h).

The two halves differ a lot in cost

diag_mat (extract a diagonal) is nearly free. new_diag_mat (src/atoms/affine/diag_mat.c:27) is already implemented as an index node — new_index(child, n, 1,indices, n) over indices[i] = i * (n + 1). Supporting k is just a different index list and length: for an (n, n) input, n - |k| entries at

indices[i] = (i + MAX(0, -k)) + (i + MAX(0, k)) * n;

No new sparsity machinery.

diag_vec (build a diagonal matrix) needs a vtable change. new_di/diag_vec.c:102) hardcodes an n×n output and places element i at flati * (n + 1). With an offset the output becomes m×m for m = n + |k|, and element i lands at

The Jacobian path goes through the diag_vec_alloc hook (include/utils/matrix.h:106,155, used at src/atoms/affine/diag_vec.c:54), which is implemented per matrix type:

  • src/utils/sparse_matrix.c:268
  • src/utils/permuted_dense.c:342
  • src/utils/stacked_pd.c:285

Each needs to place child row i at the offset output row rather than

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions