Skip to content

Record the MLX convergence-stop divergence - #249

Merged
neuromechanist merged 2 commits into
devfrom
248-mlx-divergence-docs
Aug 16, 2026
Merged

Record the MLX convergence-stop divergence#249
neuromechanist merged 2 commits into
devfrom
248-mlx-divergence-docs

Conversation

@neuromechanist

Copy link
Copy Markdown
Member

Documentation half of #248 (the implementation stays open).

What

docs/guides/amica-differences.md's backend table covered Newton, PDF families,
component sharing, outlier rejection, precision and rank detection — but not the
convergence stops. MLX implements neither:

grep target hits in pamica/mlx_impl/core.py
min_dll, use_min_dll, maxincs 0
min_nd, use_grad_norm, ndtmpsum 0

Its only stop_reason values are max_iter (:570), lrate_floor (:659) and the
degenerate ones (:590, :634). An MLX fit always runs the full iteration
budget
, while the other backends stop on min_dll (on by default at 1e-9) at
iteration 326-1076 on the bundled sample.

Why this one needs saying

The doc previously closed with "MLX limitations raise NotImplementedError
rather than differing silently." Verified by probing the constructor — that is
true for what it covers:

passed to AMICAMLXNG(...) result
do_newton=True NotImplementedError
min_dll, min_nd, use_grad_norm, keep_best, do_reject TypeError (no such parameter)

But a stop that does not exist has no parameter to reject, so nothing signals
its absence. That is the gap the section now names, with the practical
consequence: pick max_iter for MLX from where the other backends actually
converge on your data, not from their default budget.

It also scopes a claim in validation.md — "min_nd is unreachable in all
three implementations" means Fortran, PyTorch and NumPy. MLX never computes the
quantity, so it cannot be evaluated on that axis.

Also corrected

The keep_best / MIR / persistence rows were added and verified per backend
rather than assumed — NumPy has no keep_best, no mir, and no state_dict
(it persists through the EEGLAB amicaout format instead), so an earlier draft
calling it "partial" would have been wrong.

Docs-only; no code paths touched.

neuromechanist and others added 2 commits August 15, 2026 23:32
The backend table listed Newton, PDF families, sharing, rejection, precision
and rank detection, but not the convergence stops -- and MLX implements
neither. It has no min_dll/use_min_dll/maxincs and no
min_nd/use_grad_norm/ndtmpsum, so its only outcomes are max_iter,
lrate_floor and the degenerate reasons: an MLX fit always runs the full
budget while the other backends stop on likelihood stagnation.

This is the one MLX limitation that does not fail loudly. Unsupported
parameters are absent from the constructor (TypeError) and do_newton /
non-GG pdftype raise NotImplementedError, but there is no parameter to
reject for a stop that simply does not exist. Also correct the keep_best /
MIR / persistence rows, which differ per backend rather than tracking MLX.

Refs #248.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two review findings, same root cause: the section was written against
PR #244's validation.md rewrite, which is not on dev.

It cross-referenced a claim ('in all three implementations') that exists
only on that branch, so merging in the wrong order would have made the
reference false. Dropped; the point is now stated without asserting what
another page says.

It also framed 'always runs the full budget' as unique to MLX, citing the
iteration 326-1076 min_dll measurement. But PyTorch reaches its own
max_iter=100 default first, so that contrast was wrong in the same way the
validation.md table was before #244 corrected it. The real distinction is
what raising max_iter buys: on the other backends a larger budget lets the
likelihood stop end the fit, on MLX it is simply spent. Also name
numpy_impl's min_grad_norm in the row and the prose.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@neuromechanist

Copy link
Copy Markdown
Member Author

Both Critical findings confirmed and fixed in f2ca8d7. They had a single root cause, correctly identified: this section was drafted against #244's validation.md rewrite, which is not on dev.

Critical 1 — cross-reference to content that does not exist on the target branch. Correct. The phrase "in all three implementations" lives only on 218-document-min-nd. Rather than sequence the two PRs, I made this section self-contained: it no longer asserts what another page says, so merge order no longer matters. Verified there is now no reference to validation.md, "three implementations", or the 326-1076 figure in this file.

Critical 2 — the 326-1076 framing implied MLX is uniquely budget-bound. Correct, and worth stating plainly: this is the same error #244 exists to fix, reintroduced one file over. PyTorch reaches its own max_iter=100 default long before min_dll can fire, so "MLX always runs the full budget" was not a contrast at all. Rewritten around the distinction that actually holds — what raising max_iter buys you: on the other backends a larger budget lets the likelihood stop end the fit once it stops improving; on MLX a larger budget is simply spent. No measurement is cited now, so nothing here depends on an unmerged source.

Important 4 — min_nd vs min_grad_norm. Added in both the table row and the prose, so a numpy_impl user grepping for min_nd is not left concluding the feature is absent rather than renamed.

Thanks for checking the per-backend cells independently rather than taking the table's word — the keep_best/MIR/persistence rows were ones I'd verified after an earlier draft got NumPy wrong, and it is useful to have that confirmed from a second read.

@neuromechanist
neuromechanist merged commit dc6c47a into dev Aug 16, 2026
1 check passed
@neuromechanist
neuromechanist deleted the 248-mlx-divergence-docs branch August 16, 2026 06:42
Sign up for free to 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