perf: apply the delete step's 2x2 with BLAS rot - #12
Merged
Conversation
qr_delete's chase applies a 2x2 reflection to a pair of Q's columns once per step. Spelled out in NumPy that allocates two n-vectors per call; BLAS rot does the same work in place, writing through to Q's Fortran-ordered buffer. BLAS has no 2x2 reflection, only the rotation, whose second output is the exact negation of the reflection's -- so one sign flip, exact in IEEE, recovers the reference's convention. The guard on contiguity is load-bearing rather than defensive: handed a strided view, f2py copies and silently drops the overwrite, which is the same hazard _reflect already handles for dger. 2.0-3.0x on the pairwise update, and 5-8% per solve on problems whose working set churns; inert where no constraint is ever dropped. Iteration counts and active sets are unchanged -- all 867 differential tests against the C reference still pass, as do the QR invariants. The paper records the finding, including why no single-transformation form exists for the deletion, and that the rotation-without-the-flip variant was implemented and verified against Proposition 1 before being rejected for measuring no faster and forfeiting the reflection's symmetry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
qr_delete's chase applies a 2×2 reflection to a pair ofQ's columns once perstep. Spelled out in NumPy that allocates two
n-vectors per call:BLAS
rotdoes the same work in place, writing through toQ's Fortran-orderedbuffer. Only
_mixand a new_ROThandle change —qr_delete,_reflection_2x2and_swapare byte-identical tomain.Why the contiguity guard is load-bearing
BLAS has no 2×2 reflection, only the rotation, whose second output is the exact
negation of the reflection's — so one sign flip (exact in IEEE) recovers the
reference's convention.
The guard is not defensive boilerplate. Handed a strided view, f2py copies and
silently drops the overwrite, returning a correct-looking array while leaving
the original untouched — a wrong answer with no error. Verified directly. It is
the same hazard
_reflectalready handles fordgerat_qr.py:146. In thesolver
Jis always Fortran-ordered, so the fast path is what production takes(instrumented: 5516
drotcalls, zero fallbacks); the fallback is covered bytest_qr.py, which passes non-contiguous arrays.Measurements
drot2.0–3.0× on the pairwise update itself. Inert on problems that never drop a
constraint — the first benchmark I ran showed no gain at all, because profiling
later established it triggered zero deletions.
_mixcall counts are identical between the two versions (758 / 3976 / 14970),an independent check that the active-set path did not shift.
Correctness
return values including iteration counts and the active set, so the
active-set path is unchanged, not merely the minimiser.
test_qr.py(66 tests) checks both invariants directly:J Jᵀ = G⁻¹andJᵀ A = [[R],[0]]._qr.pyat 100% statement and branch coverage, 0partial.
ruff check,ruff format --check,mypy --strict,interrogate(100%floor): all pass.
Paper
docs/paper/quadprog.tex, §sec:invariance— the deletion passage previouslyasserted in three lines that "no analogous single-transformation form exists". It
now gives the reason (a Householder annihilates all but one component of a
single vector; the bulge left by a removed column is a subdiagonal spread
across distinct columns, so no one reflection reaches it), records the BLAS
routing and the numbers above, and states the failed first measurement — the
paper already reports negative results in full, so this fits its existing habit.
It also adds a second, independent corroboration of Proposition 1 on the
deletion side. Proposition 1 implies the sign flip is unnecessary: using the
rotation outright merely replaces
RbyS_k RandJbyJ S. That variantwas implemented and verified — every iteration count reproduced,
|R|and|J|elementwise unchanged, only signs differing — then rejected, because it
measures no faster (the flip is one in-place pass) and forfeits the reflection's
symmetry, which is what lets one 2×2 serve both
J's columns andR's rows.Compiles clean under
tectonic: no errors, no undefined references, no LaTeXwarnings.
Not touched
The performance table at
quadprog.tex:583. I could not verify whether the7.1× at n=700 moves. My box-constrained reconstruction showed zero deletions, but
ran 7.25 ms against the paper's 46 ms — plainly not the same problem set, so it
proves nothing. Editing that table off my own numbers would be worse than leaving
it. Expectation is that it is unchanged, but that is a guess and the table should
not rest on one.
The abstract's "Three findings." This is a 5–8% gain in the same BLAS-routing
family as finding one, not a fourth finding.
🤖 Generated with Claude Code