fix: OptimisationState limits guard, VariableData.any, redundant limits overrides - #1532
Merged
Merged
Conversation
…ts overrides Three items left behind by PyAutoFit#1527, plus one live bug found while doing the first. VariableData.any dispatched through var_all ------------------------------------------- `VariableData.any` reduced via `var_all`, making it "is there a variable whose elements are ALL True" rather than "is ANY element True". For an array like `[True, False]` it answered False. That is a correctness bug in `OptimisationState.valid`, which asks `(parameters < lower_limit).any()`: a parameter vector with SOME components outside their limits was reported valid, and only a variable violating on EVERY component was caught. `MeanField`'s `valid.any()` under-reported the same way. Both call sites want a real `any`; the other four `.any()` call sites in the library are on numpy arrays and are untouched. OptimisationState.valid guards on truthiness -------------------------------------------- `if self.lower_limit and ...` now `is not None`. `lower_limit`/`upper_limit` are VariableData (a dict keyed by free variable) or None, so the old form worked only by accident of dict truthiness being non-emptiness. #1527's follow-up list read it as a scalar test and recorded a `0.0` bug that does not exist — but the guard was one type change away from making it real. Behaviour-preserving: the only case whose guard differs is the empty VariableData, where the comparison is empty and `.any()` is False either way. Redundant limits overrides removed ---------------------------------- #1527 made `Prior.limits` derive from `lower_limit`/`upper_limit`, leaving the `UniformPrior`, `LogUniformPrior` and `TruncatedGaussianPrior` overrides exact duplicates. The base coerces with `float()` and the overrides did not, so this was a possible type change — measured, and it is not one: all three store Python floats already, and under `jax.jit` a prior never reaches `limits` anyway (`tree_unflatten` -> `__init__` calls `float()` on the tracer and raises first, with or without this change). Now-unused `Tuple` imports dropped. Tests ----- `OptimisationState.valid` and `VariableData.any` had NO test coverage, so the suite would have stayed green through any change to either — the process lesson from #1477. New test_autofit/graphical/test_optimisation_state_valid.py covers both, verified by inversion: reverting `var_any` to `var_all` fails 3 of the 8. Full suite: 2186 passed, 36 skipped (baseline 2178/36, +8 new). Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Aug 27, 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
Three items PyAutoFit#1527 left behind, plus one live bug found while doing the first.
VariableData.anyreduced throughvar_all. It meant "is there a variable whose elements are all True" rather than "is any element True" — forarray([True, False])it answeredFalse.OptimisationState.validasks(parameters < lower_limit).any(), so a parameter vector with some components outside their limits was reported valid; only a variable violating on every component was caught.MeanField'svalid.any()under-reported the same way.That, not the truthiness guard #1527's follow-up list named, is why the Laplace limits check under-enforced.
OptimisationState.validguarded on truthiness.if self.lower_limit and …is nowis not None. Both limits areVariableData(adictkeyed by free variable) orNone, so the old form worked only by accident of dict truthiness being non-emptiness. #1527 read it as a scalar test and recorded a0.0bug that does not exist — but the guard was one type change away from making it real.Three
limitsoverrides were exact duplicates. #1527 madePrior.limitsderive fromlower_limit/upper_limit, leavingUniformPrior,LogUniformPriorandTruncatedGaussianPriorrestating the base. Removed, with their now-unusedTupleimports.Closes #1531.
API Changes
No public API changes.
Prior.limitsreturns the same values and the same types for every prior family; the three deleted overrides were already returning what the base returns.One behaviour change, and it is the bug fix:
VariableData.any()now returnsTruewhere any element isTrue, rather than only where some variable's elements are allTrue. Two consumers see it —OptimisationState.valid(which now rejects partial limit violations, as intended) andMeanField's validity count (which now reports partial validity). Both are corrections. The library's other four.any()call sites are on numpy arrays and are unaffected.Test Plan
NUMBA_CACHE_DIR=… MPLCONFIGDIR=… python3 -m pytest -x -q test_autofit/— 2186 passed, 36 skipped (baseline onmain: 2178 / 36, so +8 and no regressions).test_autofit/graphical/test_optimisation_state_valid.py— 8 tests.OptimisationState.validandVariableData.anyhad no coverage at all before it, so the suite would have stayed green through any change to either.var_anytovar_allfails 3 of the 8, including both partial-violation cases.pytest -x, GREEN on a clean tree.Measured, not argued
limitsdeletion is not a type changefloats, so the base'sfloat()is a no-op — values and types identical across all five prior familiesjitregression eitherjax.jita prior never reacheslimits:tree_unflatten→__init__callsfloat()on the tracer and raisesConcretizationTypeErrorfirst, with and without this changeVariableData— falsy before (comparison skipped), truthy now (comparison run,.any()isFalse).validreturnsTrueeither wayMeanField's.any()is the only otherVariableDataconsumer.any()call site inautofit/; the rest are numpyScope
This PR covers two PyAutoMind prompts rather than one, against the usual one-prompt-one-PR rule — both are the same cleanup left by #1527, and the second is three line deletions:
draft/refactor/autofit/optimisation_state_limit_guard_truthiness.mddraft/refactor/autofit/redundant_prior_limits_overrides.mdIt also widens past both to fix
VariableData.any, without which the guard being tidied does not actually enforce anything. Happy to split either way if a reviewer prefers.Not done here
check_limitsEP question. The prior declares(0, inf)while its message stays at±inf, soMeanField.lower_limithandsOptimisationStatea-inffor a strictly positive parameter. Measured: EP is not producing wrong results today, because the message's own density returns a clean-infat negative values rather thanNaN. The limits check is redundant for that prior, not load-bearing. Tracked indraft/research/graphical_ep/transformed_message_declares_support.md.message.logpdf(0.0)is-1.798e308(negative float max) where the prior says-inf. Noted, unverified, out of scope.Generated by Claude Code