Reset rholrate per iteration to match Fortran - #194
Conversation
Review (/review-pr, reviewers = Sonnet)Ran the pr-review-toolkit code-reviewer and pr-test-analyzer on the diff. Correctness checks were clean (ceiling-ratchet gating 1. numpy fix had no test that would fail without it (test-analyzer, sev 8) — ADDRESSED. Extended the existing fast, deterministic unit test 2. numpy 3. MLX backend has the same bug (code-reviewer) — TRACKED as #195, not folded in. 4. torch fit-start refit reset has no direct test (test-analyzer, sev 4) — pre-existing, out of scope. All added tests + existing parity gates ( |
Summary
AMICATorchNG(and the legacynumpy_implbackend) diverged from the Fortran reference in the rho (generalized-Gaussian shape) learning-rate schedule: torch permanently decayedrholrateon every log-likelihood decrease, whereas Fortran resetsrholrate = rholrate0each iteration and only ratchets the ceiling atmaxdecs. Once an LL decrease shrankrholrateit stayed shrunk (monotone decay), so on a long run the rho update was progressively under-relaxed and eventually frozen (rate collapsed to ~1e-5 within a few hundred iterations). Refs #193.Fortran expressions
amica15.f90:1788/:1795—update_paramssetsrholrate = rholrate0every iteration, before the rho update at:1814. So Fortran's per-decreaserholrate = rholrate*rholratefact(:1045) is overwritten and never reaches the rho update; the effective rate is always therholrate0ceiling.amica15.f90:1050— the ceilingrholrate0ratchets*= rholratefactonly atmaxdecs, gated oniter > newt_start(unlikenewtrate, which is additionally gated ondo_newton).Fix
rho has no per-iteration ramp, so the effective rho rate is exactly its ceiling. Both backends now carry that ceiling directly:
pamica/torch_impl/core.py):self.rholrateis the ceiling — reset torholrate0at fit start (existing), ratcheted only in themaxdecsblock (gatedit > newt_start), never per-decrease. The buggy per-decreaseself.rholrate *= self.rholratefactis removed.pamica/numpy_impl/core.py): same —self.rholrateis the ceiling, reset torholrate0at fit start, ratcheted atmaxdecs(the previously-deadrholrate0ratchet is retargeted torholrate); the per-decrease decay is removed.rholrate0stays the pristine constructor value in both. No state-dict format change (the persistedrholratefield now carries the ceiling; round-trip is unchanged).Parity validation (vs
amica15mac, bundled 32ch/30504 data, 2000 iters, do_newton)Converged rho distribution (shape param, bounded [1,2]) — sorted, permutation-free:
Sorted-rho L1 distance: torch-vs-Fortran = 0.032, Fortran-vs-Fortran = 0.018 — torch's rho distribution sits within Fortran's own run-to-run spread on this data. Post-fit
rholrateends at 0.00039 (=0.05 * 0.5**7, sevenmaxdecsratchets, matchingnewtrate), not the buggy per-decrease collapse (0.05 * 0.5**19 ~ 1e-7).Note on #145
This is a correctness/parity fix on its own merits; it is evaluated against the Fortran binary, NOT the #145 long-budget Newton metric. As documented in #193, matching Fortran here worsens the #145 min-correlation on the (data-inadequate, k=29.8) bundled recording, because the accidental decay was freezing rho and removing a Newton noise source. That is expected and is a red herring for #145 (see the #145 investigation notes).
Test plan
test_rholrate_ratchets_at_maxdecs_not_per_decrease(marked slow): on the sample EEG a 300-iter Newton run overshoots and triggers several LL decreases; asserts the survivingrholrateratcheted exactly as often asnewtrate(both gated oniter > newt_startatmaxdecs), and is orders of magnitude above the old per-decrease-decay value.test_full_fit_parity_numpy_vs_ng,test_end_to_end_correlation_vs_fortran,test_sample_data.py(numpy-vs-Fortran), torch/numpy non-slow suites,ruff,ty.Refs #193