Skip to content

Replace OPR's SVDs with Gram-matrix factorizations - #96

Merged
ShantanuKodgirwar merged 2 commits into
mainfrom
opr-gram-orthogonalization
Aug 3, 2026
Merged

Replace OPR's SVDs with Gram-matrix factorizations#96
ShantanuKodgirwar merged 2 commits into
mainfrom
opr-gram-orthogonalization

Conversation

@ShantanuKodgirwar

Copy link
Copy Markdown
Contributor

Last of the series. Follows #90, #91, #93, #94 and the benchmark PR.

This changes default numerical output — hence 0.2.80.3.0 rather than a patch bump.

Why

Two thirds of an OPR iteration is linear algebra, not ptychography:

stagems/itershare
orthogonalizeIncoherentModes199.939.6%
orthogonalizeProbeStack133.826.5%
position loop and everything else170.833.9%

And probe_stack grows linearly in frames and modes — 10.4 GB at 512 px / 890 frames /
6 modes, before the transient the SVD needs on top. On a 32 GB card that is the binding
constraint, so this is as much a memory change as a speed one.

What changed

orthogonalizeProbeStack ran a full SVD of the (Np², nFrames) stack, allocating a
(Np², nFrames) U it then throws most of away. gram_tsvd gets the same rank-k subspace from
the (nFrames, nFrames) Gram matrix instead.

orthogonalizeIncoherentModes ran one small SVD per frame in a Python loop. The batched
form factorizes all frames at once, in frame chunks so the transpose scratch stays bounded
rather than allocating a second copy of the whole stack.

364 px, 202 fr, 4 modes512 px, 890 fr, 6 modes
orthogonalizeIncoherentModes4.1x1.8x
gram_tsvd vs full SVD1.8x, 5.3x less peak memory1.9x, 4.3x less

The batched advantage shrinks with size — the loop's cost is mostly per-frame launch overhead,
which is exactly what batching removes; at large sizes the factorization itself dominates.

End to end, 364 px / 202 frames / 4 modes over 30 iterations:

variantms/iterpeak GBspeedupmemory
numpy full SVD (previous default)518.83.181.00x
OPR_tsvd_type="gram" only452.51.671.15x1.90x
OPR_fast_orthogonalization only360.93.361.44x0.95x
both (new defaults)296.31.851.75x1.72x

svd on the Gram matrix, not eigh

Mathematically identical — for a Hermitian positive semi-definite matrix the SVD and the
eigendecomposition coincide, and svd returns descending order for free. The practical reason is
that cupy.linalg.eigh is not importable in every CuPy/CUDA install (it routes through
cupyx.cusolver; here it fails on libcusolver.so.11). The implementation deliberately does not
depend on it.

On the numerics — please read this bit

A naive elementwise comparison of the probe reports relative error 2.0, which looks
catastrophic and is not. Mode vectors are defined only up to a per-mode global phase, and the
Gram route makes a different arbitrary choice than LAPACK. A phase flip is exactly relative
error 2.

Everything gauge-independent agrees:

quantityagreement
object1.4e-07
error metric4.6e-08
mode powers4.1e-07, 1.1e-06
|probe|8.0e-07
mode vectors, after aligning phase8.2e-07, 1.7e-04

Over a longer run the object does drift, because the solver amplifies any perturbation. At
364 px / 202 frames / 30 iterations the Gram route moves the object by 1.6e-02 — while
perturbing the initial object of the unmodified path by a relative 1e-6, below the float32
resolution of the inputs, moves it by 2.8e-02. The change sits inside the algorithm's own
sensitivity to its inputs, and the error metric agrees to 9.3e-05.

Escape hatch, and it is tested

params.OPR_tsvd_type="numpy"params.OPR_fast_orthogonalization=False

recovers the previous output. A regression golden pins that path, so the claim is verified rather
than asserted.

Tests

  • tests/Engines/test_opr_linalg.pygram_tsvd unit tests, on CPU so CI covers them even
    though OPR itself is GPU-only. Includes rank clamping and rank-deficient input.
  • tests/regression/test_opr_regression.py — the legacy golden, and the gauge-invariant
    comparison above.

Suite goes 61 → 71 passing.

Incidental fixes in the same functions

  • orthogonalizeProbeStack dispatches through getArrayModule instead of calling cp directly
  • the "numpy" path no longer mutates the caller's singular values in place
  • an unknown OPR_tsvd_type now raises, instead of failing later with an unbound name

Note

notes/gram_orthogonalization.md is a full walkthrough of why the Gram route gives the same
answer and where the speedup comes from — written for someone who has not thought about this
before.

ShantanuKodgirwarand others added 2 commits August 3, 2026 14:16
Two thirds of an OPR iteration is linear algebra rather than ptychography, and
probe_stack grows linearly in frames and modes -- 10.4 GB at 512 px / 890
frames / 6 modes, before the transient the SVD needs on top. On a 32 GB card
that is the binding constraint, so this is as much a memory change as a speed
one.
Both orthogonalization steps are replaced:
- orthogonalizeProbeStack ran a full SVD of the (Np^2, nFrames) probe stack,
which allocates a (Np^2, nFrames) U it then throws most of away. gram_tsvd
gets the same rank-k subspace from the (nFrames, nFrames) Gram matrix
instead: 1.8x faster on 5.3x less peak memory at 364/202, 1.9x on 4.3x less
at 512/890.
- orthogonalizeIncoherentModes ran one small SVD per frame in a Python loop.
The batched form factorizes all frames at once: 4.1x at 364/202/4, 1.8x at
512/890/6. The advantage shrinks with size because the loop's cost is mostly
per-frame launch overhead, which is what batching removes. It works in frame
chunks so the transpose scratch stays bounded rather than allocating a second
copy of the whole stack.
End to end at 364 px, 202 frames, 4 modes over 30 iterations: 518.8 -> 296.3
ms/iter (1.75x) with peak memory 3.18 -> 1.85 GB (1.72x).
Both use svd on the Hermitian Gram matrix rather than eigh. That is
mathematically the same -- for a Hermitian positive semi-definite matrix the
two coincide, and svd returns descending order for free -- but eigh routes
through cupyx.cusolver, which is not importable in every CuPy/CUDA install.
On the numerics. The Gram route reaches the same subspace but makes a different
arbitrary choice of per-mode global phase than LAPACK, so a naive elementwise
comparison of the probe reports relative error 2.0. That is a phase flip, not a
different answer. Everything gauge-independent agrees: object 1.4e-07, error
metric 4.6e-08, mode powers 4.1e-07 and 1.1e-06, |probe| 8.0e-07, and after
aligning the phase the mode vectors agree to 8.2e-07 and 1.7e-04.
Over a long run the object does drift, because the solver amplifies any
perturbation. At 364/202/30 the Gram route moves the object by 1.6e-02, while
perturbing the initial object of the unmodified path by a relative 1e-6 --
below the float32 resolution of the inputs -- moves it by 2.8e-02. The change
sits inside the algorithm's own sensitivity to its inputs, and the error metric
agrees to 9.3e-05.
Defaults change, hence the minor version bump. Set OPR_tsvd_type="numpy" and
OPR_fast_orthogonalization=False to recover the previous output; a regression
golden pins that path so the escape hatch stays tested.
Also fixes orthogonalizeProbeStack to dispatch through getArrayModule instead
of calling cp directly, to not mutate the caller's singular values in place on
the "numpy" path, and to raise on an unknown OPR_tsvd_type rather than failing
later with an unbound name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Renders the seven derivation steps as LaTeX rather than fenced code blocks,
and switches the inline occurrences to match so the note does not carry two
notations for the same thing.
Also corrects section 7, which cited test names that do not exist: the tests
are test_opr_legacy_path_golden and
test_opr_gram_matches_legacy_on_gauge_invariant_quantities, in
tests/regression/test_opr_regression.py.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ShantanuKodgirwar
ShantanuKodgirwar merged commit 5d9ad2c into mainAug 3, 2026
3 checks passed
@ShantanuKodgirwar
ShantanuKodgirwar deleted the opr-gram-orthogonalization branch August 3, 2026 12:47
Sign up for freeto 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

@ShantanuKodgirwar