Skip to content

fix(translate): drop identically-zero terms to fix the correlation regression#19

Closed
oameye wants to merge 1 commit into
sqav0.6from
fix-translate-drop-zero-terms
Closed

fix(translate): drop identically-zero terms to fix the correlation regression#19
oameye wants to merge 1 commit into
sqav0.6from
fix-translate-drop-zero-terms

Conversation

@oameye

@oameye oameye commented Jun 24, 2026

Copy link
Copy Markdown
Member

The benchmark CI was right after all: there's a real regression in the Correlations/two-time (and time-dependent interaction-picture) benchmark, and it's not cross-runner noise. I tracked it down to translate_qo.

The time-dependent path kept terms whose coefficient is identically zero once the parameters are substituted. When a coupling is set to zero, the substitution leaves a conj(0.0) factor (or conj(conj(0.0)) when the cascade Hamiltonian's second cross-term is built as adjoint(X)), and Symbolics doesn't fold either back to 0. So the term looked nonzero, survived into the compiled closure, and got re-evaluated (pulse interpolation + operator rebuild) on every single ODE step. The g_v => 0 coupling in the correlation benchmark hits this directly.

The fix folds conj of a numeric literal in each coefficient and drops the ones that collapse to zero. conj of a symbolic time variable is left alone, so genuinely time-dependent complex couplings are untouched.

Measured on the correlation benchmark (same machine, g_v = 0):

allocations memory per-call H(t)
before (current sqav0.6) 5.17M 555 MB 6128 B
after this PR 2.78M 373 MB 2080 B
SQA 0.4.5 (old baseline) 2.78M 373 MB 2400 B

So it's back to the old 0.4.5 allocation level (actually a hair below per-call). Output is bit-identical (dropping identically-zero terms is exact), numeric SLH composition is unchanged since this doesn't touch the cascade, and translate construction time is unchanged. test_translate (37) and test_correlations (4) pass locally.

Worth noting: the earlier "it's just runner noise" framing on PRs #15/#18 was wrong, this was a genuine ~1.5-2x slowdown that the minimum/gcsample work correctly surfaced instead of hiding.

…tion

The time-dependent path of `translate_qo` kept terms whose coefficient was
identically zero after parameter substitution. A coupling substituted to zero
leaves an unfolded `conj(0.0)` factor (or `conj(conj(0.0))` when a cascade
Hamiltonian's second cross-term is formed as `adjoint(X)`), which Symbolics does
not fold to zero, so the term survived and the compiled closure re-evaluated it
(pulse interpolation plus operator rebuild) on every ODE step.

Fold `conj` of a numeric literal in each coefficient, then drop the coefficients
that become zero. `conj` of a symbolic time variable is left untouched, so
genuinely time-dependent complex couplings are unaffected.

Effect on the correlation benchmark (single-photon cavity, g_v set to 0):
allocations fall from 5.17M to 2.78M and memory from 555 MB to 373 MB, both back
to the SecondQuantizedAlgebra 0.4.5 level; per-call H(t) allocation goes from
6128 B to 2080 B. Output is bit-identical (dropping identically-zero terms is
exact), numeric SLH composition is unaffected (this does not touch the cascade),
and translate construction time is unchanged. test_translate and
test_correlations pass.
@oameye oameye marked this pull request as draft June 24, 2026 12:45
@oameye

oameye commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Superseded by the proper fix in SecondQuantizedAlgebra: qojulia/SecondQuantizedAlgebra.jl#193.

The dead terms this PR dropped in translate_qo came from SQA leaving conj(conj(x)) (cascade adjoint) and conj(<const>) (a coupling substituted to zero) unfolded. SQA #193 folds both at the source, so substitute drops the zero terms before translation sees them. Verified the exact shapes here (time-dependent g_u times a conj-of-zero coupling) collapse to 0, so this QIO-side workaround is no longer needed.

Closing rather than patching QIO. Once SQA #193 is released, QIO just needs a [compat] bump to pick it up. (Reopen if we want a temporary bridge to green CI before that release.)

@oameye oameye closed this Jun 24, 2026
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.

1 participant