Remove dead normalization machinery left by the orthonormal refactor - #31
Merged
Merged
Conversation
With no path converting normalization any more, several helpers and struct
fields became unreachable:
SHTPlan.norm_tmp1 / norm_tmp2 -- two (lmax+1)x(mmax+1) ComplexF64 buffers
allocated per plan that nothing reads
DistTransposePlan.need_norm / Mloc / alm_scratch, plus _scale_alm_to_internal!
and _scale_alm_to_cfg! -- need_norm was
hardcoded false, so the machinery was inert
_scale_cotangent -- in BOTH AD extensions, no call sites left
The DistTransposePlan docstring still described the removed fields and declared
that "coefficients crossing this plan's API are in cfg's convention", which had
become the opposite of what the code does. Rewritten to state the orthonormal
contract.
Serial 67278/67278; 8 MPI suites at 4 ranks; 56 custom assertions at 8 ranks;
cross-API equality unchanged at machine precision.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
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
1 participant
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.
Follow-up to #29, which merged before this landed.
With no code path converting normalization any more, several helpers and struct fields became unreachable:
SHTPlan.norm_tmp1/norm_tmp2— two(lmax+1)×(mmax+1)ComplexF64 buffers allocated per plan that nothing readsDistTransposePlan.need_norm/Mloc/alm_scratch, plus_scale_alm_to_internal!and_scale_alm_to_cfg!—need_normwas hardcodedfalse, so the whole machinery was inert_scale_cotangent— in both AD extensions, no call sites leftThe
DistTransposePlandocstring still listed the removed fields and declared that "coefficients crossing this plan's API are in cfg's convention" — the opposite of what the code does. Rewritten to state the orthonormal contract.Verification
Serial 67278/67278; 8 MPI suites at 4 ranks; 56 custom assertions at 8 ranks. Cross-API equality unchanged at machine precision, and still identical across
:orthonormal/:schmidt/:fourpi:SHTPlansphtor == non-planSHqst_to_point== its gridpacked_cplx== dense synthesisThe adversarial review I ran against #29's final commit (
f19a2bcb, a 241-line bulk-regex deletion) did not actually complete — 4 of its 5 agents died on an API session limit, so it produced zero candidates and reported "no findings" vacuously. That commit merged without an independent review pass.That matters because one regex in that commit had already silently deleted live code (
analysis_qstlost its transform calls; caught only by anUndefVarErrorin the suite). The test suite is green and I checked the obvious failure modes by hand, but a proper review off19a2bcbis still owed.🤖 Generated with Claude Code