Skip to content

Add canonicalization backend comparison benchmarks#32

Open
mellowyellow71 wants to merge 2 commits into
cvxpy:mainfrom
mellowyellow71:backend-canonicalization-benchmarks
Open

Add canonicalization backend comparison benchmarks#32
mellowyellow71 wants to merge 2 commits into
cvxpy:mainfrom
mellowyellow71:backend-canonicalization-benchmarks

Conversation

@mellowyellow71

@mellowyellow71 mellowyellow71 commented Jul 6, 2026

Copy link
Copy Markdown

This PR adds benchmark/canonicalization_backends.py with four benchmark
classes, all parameterized over ["SCIPY", "COO", "CPP", "RUST"]:

  • BackendCompileCanonicalization — end-to-end get_problem_data for 16
    cases covering the affine atom families (matmul/multiply/divide, rmul +
    promote, hstack/vstack, concatenate, diag/trace/kron with both kron_r and
    kron_l, convolve, indexing/transpose/reshape), ND arrays and broadcasting,
    einsum, deep and wide expression trees, a parameterized LP (the COO
    backend's native workload), a cone-heavy composite (norm1 + huber +
    quad_form), and murray-type dense constants in two density regimes (below
    and above SPARSE_DENSITY_THRESHOLD, so sparsification heuristics can be
    observed separately from the dense path).
  • BackendBuildMatrixCanonicalization — the same cases, but timing only
    the backend's build_matrix call on captured LinOp trees, isolating backend
    cost from the rest of the compilation chain.
  • DeepExpressionTreeScaling / WideExpressionTreeScaling
    get_problem_data as tree depth (-(-(...-(x))), depth 4→256) and sum
    width (n matmul terms, 8→256) grow.

Validation: asv check passes; ruff (E,F,I, line-length 100) clean; on a
local CVXPY master build with all four backends available, every case×backend
cell runs, and all four backends produce numerically identical stuffed
tensors on every case (max abs diff 0.0, parameter slices included).

Follows the precedent of backends/dpp_canonicalization.py for backend
comparison benchmarks, but inside benchmark_dir so asv discovers it.


Changes in response to review (@PTNobel)

Comment-by-comment:

  • "LinOp trees" → DAG (docstring): reworded to "LinOp DAG" and cited Memoize shared LinOps in Python canonicalization backends cvxpy#3423.
    Also added a shared_subexpressions case (a reused A @ x feeding many slices) so the
    memoization in #3423 will be visible on the dashboard once it merges.
  • _reshape wrapper (order= exists since 2020): removed; direct cp.reshape(..., order="F").
  • _vec wrapper: removed; cp.vec(..., order="F") / Expression.flatten(order="F") used directly.
  • **_flatten — replaced with Expression.flatten(order="F").
  • "contract" wording: helper renamed to _skip_if_cpp_unsupported; the comment now says
    it's the same check get_problem_data performs.
  • Old CanonBackend.get_backend fallback / wrapper function: removed;
    cvxpy.lin_ops.backends.get_backend is imported directly.
  • **max(captured, ...) — Compilation invokes get_problem_matrix twice (objective, then
    constraints); on ties max(..., key=len) kept the first — the trivial objective call — so
    several build_matrix cells were timing objective stuffing. Fixed to select the constraints
    call (constr_length key) with a comment explaining the two calls; the affected cells now
    show real case-dependent spreads.
  • cp.conv fallback (deprecated 2022): removed; cp.convolve directly.
  • ** (sum tree):** checked empirically — repeated +
    flattens into a single AddExpression (8 terms → 8 args), so the LinOp sum node has width
    children rather than a chain of binary adds; the docstring now states this. The
    many-independent-constraints notion of wide is covered by the new class below.
  • Gini duplication: it is a scaled-down restatement of gini_portfolio.py's Murray —
    restated rather than imported so the pairs-matrix density is parameterizable (one case on
    each side of the sparsity threshold). Now noted in a comment above the builder.
  • Assert on SPARSE_DENSITY_THRESHOLD: went a step further — both murray regimes read
    the runtime threshold: the above-threshold case scales its density with it
    (min(3×threshold, 0.5)), and the below-threshold case reports n/a if the threshold ever
    drops beneath the natural pairs density, instead of silently measuring the wrong regime.
    One deliberate getattr(..., 0.05) default remains because released 1.9.2 predates the
    constant and this repo's CI installs PyPI cvxpy.
  • "A wide problem is many constraints" + cvxcore parallelism: added
    WideConstraintScaling (8/64/256 independent A_k @ x <= b_k constraints, each its own
    LinOp root). On cvxcore: build_matrix does have #pragma omp parallel for across
    constraints (cvxcore.cpp:177), but PyPI wheels are built without OpenMP (no flags in
    setup/extensions.py; the single USE_OPENMP=True CI job is test-only and every wheel
    step is gated on it being off), and even when enabled, every tensor write serializes under
    #pragma omp critical (the TODO at cvxcore.cpp:174 notes each thread would need its own
    ProblemData). So as shipped, OpenMP shouldn't be expected to lift CPP over SCIPY/COO on
    wide problems. Measured compile ms (SCIPY/COO/CPP at n=8/64/256, CPP built from source
    without OpenMP, same as wheels): 3.0/2.6/1.7 → 19.4/17.1/10.8 → 77/73/48.

Four ASV classes comparing the SCIPY, COO, CPP, and RUST backends on
affine atom families, expression-tree shapes, ND arrays, broadcasting,
einsum, convolve, kron variants, and dense-constant density regimes.
Unavailable backends are reported as unsupported rather than failing,
so the suite runs against PyPI cvxpy on CI today. A LINOP_COVERAGE
checklist maps every LinOp node type to the cases exercising it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@PTNobel PTNobel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good. I think you are being way too defensive for old code. I'm curious what inspired this choice

Comment thread benchmark/canonicalization_backends.py Outdated
Comment thread benchmark/canonicalization_backends.py Outdated
Comment thread benchmark/canonicalization_backends.py Outdated
Comment thread benchmark/canonicalization_backends.py Outdated
Comment thread benchmark/canonicalization_backends.py Outdated
Comment thread benchmark/canonicalization_backends.py Outdated
Comment thread benchmark/canonicalization_backends.py
Comment thread benchmark/canonicalization_backends.py
Comment thread benchmark/canonicalization_backends.py Outdated
Comment thread benchmark/canonicalization_backends.py Outdated
track SPARSE_DENSITY_THRESHOLD, add shared-subexpression and
constraint-width cases

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@PTNobel PTNobel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! I'm supportive of merging, but I'll wait a few days to see if @Transurgeon wants to review

@Transurgeon

Copy link
Copy Markdown
Contributor

Great job! I'm supportive of merging, but I'll wait a few days to see if @Transurgeon wants to review

I definitely want to review. One question, how is this different than our existing comparison benchmarks?

@PTNobel

PTNobel commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

how is this different than our existing comparison benchmarks?

It includes a number of edge cases that aren't covered (like deep expression trees, many constraint problems, and some more), is not run as part of CI, and has a much easier to read output when benchmarking backends.

I had suggested they put this together so we would have an easy place to put the problems that we were trying to improve with the new backend, etc. @mellowyellow71 feel free to share more, but that is the initial color.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants