Repository navigation
Phase A: Stabilize float32 AMICATorchNG via ufp/y guard - #78
Merged
neuromechanist merged 2 commits intoJul 8, 2026
Merged
neuromechanist merged 2 commits into
neuromechanist merged 2 commits into
Conversation
float32 diverged to NaN on the full 30504-sample data across every seed (Newton on and off), while float64 converged. Root cause: the mu denominator sbeta*sum(ufp/y) (ufp=u*fp); at a sample sitting on a mixture mean, float32 rounds the scaled activation y to exactly 0, and fp(0)=0 for every family, so that term is 0/0=NaN and one NaN summand poisons dmu_d. float64 never rounds y to exactly 0. Diagnostics ruled out summation precision (accumulating block partials in float64, and Neumaier compensated summation, did not help) and the density / responsibilities (float64 there did not help either). Only guarding the ufp/y division does. The guard (ufp / where(y==0, 1, y), contributing 0 for the measure-zero sample) is a no-op in float64 (y is never exactly 0), so single-model #24 parity stays bit-identical, and it needs no float64, so it also stabilizes the MPS/float32 path (Apple GPUs have no FP64) -- epic #74 Phase A. Tested: float32 now converges across 5 seeds x Newton on/off on the real sample EEG, matching the float64 LL to ~5 significant digits; full non-slow torch suite green (124 passed), including the NumPy-parity and byte-identity anchors. Docs updated (perf_findings, mps_pathways, AGENTS, benchmark_gpu).
- Correct the guard comment: the true ufp/y limit is NOT 0 (nonzero constant at
rho=2, integrable singularity diverging for rho<2), so the guard drops an
unrepresentable singular term, not a removable zero. Measured: it fires <=1
sample/iteration on the sample EEG (5 of 150 iters), and float32 still matches
the float64 LL to ~5 sig digits -- a bounded, negligible bias.
- Scope the "fp(0)=0 for every family" claim to the supported rho>=1 (for rho<1
the GG fp is itself NaN at 0; out of scope, default minrho=1.0).
- test: reference AMICATorchNG._DEGENERATE_STOP_REASONS instead of duplicating
the ("nan_ll","singular_ll") tuple; shorten the float64-tracking quality check
to 100 iters (it tracks float64 from iter 1; the 150-iter sweep remains the
regression guard).
- docs: finish the mps_pathways.md update -- intro to past tense, and Pathways B
and C no longer gate on Pathway A as unmet (it is done, #75).
Member
Author
Review summary (4 Sonnet reviewers: code, silent-failure, tests, comments/docs)Mutation-tested and independently verified: removing the guard fails all 5 Addressed (commit ad90407)
Considered and not changed (with rationale)
Process noteDuring the parallel review, one write-capable reviewer left a stray uncommitted |
neuromechanist
merged commit Jul 8, 2026
6d8a002
into
feature/issue-74-epic-apple-gpu
5 checks passed
This was referenced Jul 8, 2026
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.
Summary
float32
AMICATorchNGdiverged to NaN on the full 30504-sample sample EEG across every seed (Newton on and off, crashing iter ~9-105), while float64 converged. This blocked the whole Apple-GPU roadmap (epic #74): Apple GPUs have no FP64, so every MPS/MLX pathway is gated on a stable float32 AMICA.Root cause (a per-element divide-by-zero, not summation precision). The mu denominator is
sbeta*sum(ufp/y)withufp = u*fp. At a sample sitting on a mixture mean, float32 rounds the scaled activationyto exactly 0, and the scorefp(0)=0for every family, so that term is0/0 = NaNand one NaN summand poisons the wholedmu_d. float64 never landsyon exact 0, hence float64-only.Fix: a one-line guard,
ufp / where(y==0, 1, y)(contributing 0 for the measure-zero sample). It is a no-op in float64 (bit-identical, so single-model #24 Fortran parity is preserved) and needs no float64, so it also stabilizes the MPS/float32 path.Diagnosis (why not compensated summation)
The plan led with an inter-block summation-precision hypothesis; diagnostics on the real data refuted it and landed on the pre-registered fallback branch:
|y|^rho/log_pdfin float64 did not help (nor, per Stabilize float32 AMICATorchNG for the GPU fast path #70, the responsibilities).ufp/ydivision does. So the earlier "needs mixed precision, payoff ~1.5-2x" conclusion is superseded; the fix needs no float64 at all.Test plan
tests/torch_tests/test_ng_float32_stability.py: full-data float32 fit converges across 5 seeds spanning Newton on/off (finite LL, non-degenerate stop), and float32 LL matches the in-test float64 fit within 0.05 (observed: ~5 significant digits). Plus a fastfp(0)=0invariant test and an MPS smoke test (self-skips in CI).ruff check+ruff format --checkclean.Docs updated
.context/issue-63/perf_findings.md(section 4 corrected: it IS a divide-by-zero, per-element),.context/mps_pathways.md(Pathway A marked DONE, mechanism corrected),AGENTS.md,benchmarks/benchmark_gpu.py.Closes #75
Part of epic #74