fix: MultiStart gradient step loop crashes on a real iterations_per_full_update cadence - #1421
Merged
Merged
Conversation
The step loop does `for _ in range(iterations)`, but `AbstractSearch.__init__` stores `iterations_per_full_update` as a float (abstract_search.py:219) so the inf-like 1e99 config default is representable. The crash was latent because `min(1e99, steps_remaining)` returns the int operand — only a user-supplied cadence *below* the remaining budget reaches `range` and raises `TypeError: 'float' object cannot be interpreted as an integer`. Six RAL chain jobs died on it. Cast at the consumer via a new `_steps_in_chunk` helper rather than changing the shared float coercion, which every other search relies on. The helper also makes the defect testable without JAX: `_fit` needs jax, optax and a JAX-traceable Analysis, and the library suite is NumPy-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of the previous commit: `int` truncates towards zero, so a fractional cadence below 1 (e.g. iterations_per_full_update=0.5) yields `range(0)` — no steps run, `total_steps` never advances, and the enclosing while-loop spins forever re-running `perform_update`. That traded a loud TypeError for a silent hang, which on HPC burns the whole allocation. Floor at 1: the slowest cadence that still progresses, and it can never overshoot `steps_remaining` (>= 1 whenever the loop runs). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follows an adversarial review of the previous commit. `max(1, int(...))` silently laundered invalid input: -5, 0.5 and 50.9 all became a plausible-looking cadence, so a typo would quietly run a schedule the user never asked for. Validate instead, and name the bad value. The same review showed the "chunk can never overshoot steps_remaining" claim held only because `n_steps` is an integer — the loop guard proves steps_remaining > 0, not >= 1, so a float n_steps=2.5 leaves a 0.5 remainder that truncates to a zero-length chunk and hangs. n_steps is annotated `int` but never validated, so it is checked too; both checks together restore the invariant the floor was papering over. Also adds a wiring guard: every other test drives _steps_in_chunk directly, so all of them would still pass if _fit went back to computing the chunk inline — the exact regression this fix is about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #1420.
Summary
The MultiStart gradient step loop does
for _ in range(iterations), butAbstractSearch.__init__storesiterations_per_full_updateas a float(
abstract_search.py:219) so the inf-like1e99config default isrepresentable. The crash was latent because
min(1e99, steps_remaining)returnsthe
intoperand — only a user-supplied cadence below the remaining budgetreaches
rangeand raises:Six RAL chain jobs (331182–331190) died on this back-to-back during the wsdev#117
Pix-Prodigy CPU campaign, using
iterations_per_full_update=50withn_steps=3000.The cast is applied at the consumer, via a new
AbstractMultiStartGradient._steps_in_chunk, rather than in the sharedfloat()coercion — that coercion exists to accept
1e99and is used by every othersearch, so changing it there is the regression-prone option. The helper also
makes the defect testable without JAX:
_fitrequiresjax,optaxand aJAX-traceable
Analysis, and the library unit suite is NumPy-only.Commits 2 and 3 come from review, and are the interesting part of this PR.
Commit 2 fixed a defect in commit 1:
inttruncates towards zero, so afractional cadence below 1 gives
range(0)— no steps run,total_stepsneveradvances, and the enclosing
whilespins forever re-runningperform_update.Commit 1 alone would have traded a loud
TypeErrorfor a silent hang, which ona cluster burns the whole allocation. It floored the chunk at 1.
Commit 3 replaced that floor, after an independent adversarial review (Codex
gpt-5.6-sol) made two points that hold up:max(1, int(...))silently launders invalid input —-5,0.5and50.9all became a plausible-looking cadence, so a typo would quietly run a schedule
the user never asked for. That is the silent-guard pattern this codebase
removes rather than adds. It now raises a
ValueErrornaming the value.steps_remaining" claim held only becausen_stepsis an integer. The loop guard provessteps_remaining > 0, not>= 1, so a floatn_steps=2.5leaves a0.5remainder that truncates to azero-length chunk and hangs.
n_stepsis annotatedintbut never validated,so it is validated too. The two checks together restore the invariant the
floor was papering over, which is why the floor could be removed rather than
kept alongside them.
API Changes
None. The only new symbol is the private
AbstractMultiStartGradient._steps_in_chunk; no public class, method, signature,argument, default or config key changed. Behaviour on the default path is
bit-identical — with the packaged
1e99cadence the expression already returnedthe
intsteps_remaining, andint()/max(1, ...)leave that untouched.Downstream workspaces and notebooks need no migration.
The change is purely corrective: inputs that previously crashed now work.
Nothing that previously ran behaves differently.
Testing
pytest test_autofit/ -xin the task worktree → 1541 passed, 1 skipped.test_autofit/non_linear/search/mle/test_multi_start_gradient.py: a realcadence below the budget returns an
intthatrange()consumes; the chunkclamps to
steps_remaining; the1e99default still gives a singlewhole-budget chunk; a falsy cadence falls back to
n_steps; an unusablecadence (
0.5,50.9,-5) and an unusablen_steps(2.5,0,-10)each raise
ValueError._steps_in_chunkdirectly, so all of them would still pass if_fitwentback to computing the chunk inline — the exact regression this PR is about.
_fitcan't be executed from this suite (jax + optax + a JAX-traceableAnalysis; the library suite is NumPy-only), so the call site is asserted atthe source level.
int()cast fails exactly one new test(
test__steps_in_chunk__real_cadence_is_an_int_range_can_consume).mainfirst:range(min(50.0, 3000))raises the reportedTypeError.Validation checklist
pytest test_autofit/ -x: 1541 passed, 1 skipped. No public APIchange, so downstream dependent suites are n/a.
smoke_tests.txtacross all six workspaces with the taskworktree's
activate.shsourced: 50 passed, 7 failed, 3 skipped/missing.All 7 failures are pre-existing — re-running each against
mainunderidentical conditions reproduces exactly the same 7 (they are the
jax_likelihoodparity scripts, which the smoke profile runs withPYAUTO_DISABLE_JAX=1). A further 5 scripts failed only in the parallelsweep and pass on the branch when re-run sequentially, matching
main—contention over shared
output//dataset/state in the runner, not aregression.
pass 1 (the truncation-to-zero hang → commit 2) and CLEAN on pass 2. An
independent adversarial review (Codex
gpt-5.6-sol, xhigh) then producedcommit 3, corrected the follow-up list below, and surfaced two pre-existing
bugs now filed separately.
2026-07-27T12:11:07Z, no RED reasons.The reason set was acknowledged by the human at this launch, verbatim:
workspace validation not passing (13 failed, 2026-07-21T19-05-22Z);33 stale parked script(s);manifest drift: tenant firewall (organ code) — 5 mismatch(es) vs PyAutoMind/repos.yaml;and the stale-tier
release validation stale: source moved since rehearsal (PyAutoNerves, PyAutoFit, PyAutoArray, PyAutoGalaxy, PyAutoLens).None touches
autofit/non_linear/search/mle/.Follow-ups (deliberately not in this PR)
mcmc/emcee/search.py:206(EnsembleSampler.sampledoesrange(iterations)with no cast) and
mcmc/blackjax/nuts/search.py:291(jax.random.split(key, 50.0)raises the sameTypeError). An independent adversarial review(Codex
gpt-5.6-sol) checked every consumer empirically and corrected anearlier, wider claim of mine:
mcmc/zeus/search.py:242is safe (zeus castsinternally via
self.nsteps = int(iterations)), andmle/bfgs/search.py:171,nest/dynesty/.../abstract.py:365andnest/nautilus/search.py:477aretolerated (comparison limits / arithmetic only). See issue fix: MultiStart gradient step loop crashes on a real iterations_per_full_update cadence #1420 for the
verified table.
sites and filed as PyAutoMind drafts:
perform_updateruns twice —_fitemits one withduring_analysis=Falseatsearch.py:432, thenstart_resume_fitemitsthe same at
abstract_search.py:704. Every other search(
emcee:238,bfgs:219,nautilus:438) passesduring_analysis=Trueunconditionally inside
_fit, so MultiStart is the outlier and doublesthe cost of final output, latents, visualization and profiling.
stop_reason="max_steps"search resumed with a largern_stepskeeps the stale stop reason in every intermediate checkpoint(
search.py:413-416only reassigns on convergence or the new ceiling).post-construction int overwrite in
autolens_workspace_developer/searches_minimal/pix_prodigy.py(branchfeature/pix-prodigy-cpu), which belongs to the livepix-prodigy-cputask(wsdev#117).
🤖 Generated with Claude Code