fix: TEST_MODE bypass must evaluate a point the model's assertions accept - #1520
Merged
Merged
Conversation
…cept The `PYAUTO_TEST_MODE=2/3` bypass has no sampler, so it picks its own evaluation point: the prior medians. A model whose components share priors and carry an ordering assertion — the standard idiom for breaking exchange degeneracy, e.g. PyAutoCTI trap models with `trap_0.release_timescale < trap_1.release_timescale` — ties exactly there, so `check_assertions` rejects it with `FitException` and the run hard-fails on an artifact of the bypass's own choice of point. A real search absorbs this by resampling. The failure had three sites, not one: - `_fit_bypass_test_mode` instantiated the model outside the `try` that catches `FitException`, so the assertion rejection escaped that guard; - every fake sample is the median vector scaled uniformly, which preserves an ordering tie, so no stored sample was reconstructible either; - `SamplesSummary.max_log_likelihood` is `@to_instance()` with `recover="raise"`, so `result.max_log_likelihood_instance` raised `SamplesException` — meaning `PYAUTO_TEST_MODE=3` was broken too, even though it never calls the likelihood. Fix all three at the source by making the bypass evaluate *and store* a vector the model accepts. `_test_mode_valid_parameter_vector` factors out the deterministic search TEST_MODE=1 recovery already used — prior medians first, then `default_rng(seed=0)` prior draws, each candidate validated — and both call sites now share it. The fixed seed keeps smoke runs reproducible. Mode 1's stronger per-sample validation is preserved via the `validate` hook. The likelihood-call guard is untouched: a pathological likelihood is a different contract from an invalid instance, and both are now covered. Closes #1519
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
The
PYAUTO_TEST_MODE=2/3bypass has no sampler, so it picks its own evaluation point: the prior medians. A model whose components share priors and carry an ordering assertion — the standard idiom for breaking exchange degeneracy, e.g. PyAutoCTI trap models withtrap_0.release_timescale < trap_1.release_timescale— ties exactly there, socheck_assertionsrejects it withFitExceptionand the run hard-fails on an artifact of the bypass's own choice of point. A real search absorbs this by resampling.The failure had three sites, not the one reported:
_fit_bypass_test_modeinstantiated the model outside thetrythat catchesFitException, so the assertion rejection escaped that guard.SamplesSummary.max_log_likelihoodis@to_instance()withrecover="raise"(it extendsSamplesInterfacedirectly, so it does not inheritSamples' next-valid recovery), soresult.max_log_likelihood_instanceraisedSamplesException;Result.instancecatches onlyAttributeError.Consequence:
PYAUTO_TEST_MODE=3was broken too, even though it never calls the likelihood — it survived the fit and died at the firstresult.max_log_likelihood_instance.This fixes all three at the source by making the bypass evaluate and store a vector the model accepts.
_test_mode_valid_parameter_vectorfactors out the deterministic search theTEST_MODE=1recovery path already used — prior medians first, thennp.random.default_rng(seed=0)prior draws, each candidate validated — and both call sites now share it. The fixed seed keeps smoke runs reproducible without touching global random state.Mode 1's stricter per-sample validation is preserved via the helper's
validatehook, so its behaviour and its exact failure message are unchanged. The likelihood-call guard is untouched: a pathological likelihood is a different contract from an invalid instance, and both are now covered.Unblocks
autocti_workspacesmoke coverage ofmodeling/start_here.py-class scripts (CTI resurrection epic, Phase 5), which currently documents this artifact in itsAGENTS.mdas a workaround.API Changes
None — internal changes only. No public symbol is added, removed, renamed, or re-signatured;
_test_mode_valid_parameter_vectoris private.There is one observable behaviour change worth a reviewer's eye: the bypass now instantiates the model once under
PYAUTO_TEST_MODE=3as well as=2(previously mode 3 did no instantiation at all). That is what makes the stored vector valid. It means a model whose constructor raises a non-FitExceptionat the medians now fails at fit time rather than at result-access time — the same failure, surfaced earlier. Cost is one instantiation, not one per sample: the 50,000-sample bypass test is unaffected.See full details below.
Test Plan
mainfirst — mode 2:FitException: 1 assertions failed!during the fit; mode 3: fit completes, thenSamplesExceptionatresult.max_log_likelihood_instance.8.1513753680827, 9.13628021504944) — confirming determinism across modes.pytest test_autofit/non_linear/search/test_abstract_search.py— 36 passed (31 before, 5 new).pytest test_autofit— 2016 passed, 34 skipped, 0 failed.New regression coverage in
TestBypassToleratesAssertionTies: a tied ordering assertion completing the fit and reconstructingresult.max_log_likelihood_instanceat modes 2 and 3; cross-mode determinism of the chosen point; and an unsatisfiable assertion still failing cleanly within the attempt budget, chained to the final rejection.Full API Changes (for automation & release notes)
Removed
None.
Added
None public. Internal:
NonLinearSearch._test_mode_valid_parameter_vector(model, failure_prefix, validate=None)— returns(parameter_vector, validated)for the first deterministically-drawn vector the model accepts; raisesFitExceptionchained to the final rejection onceTEST_MODE_REPRESENTATIVE_MAX_ATTEMPTSis spent.Renamed
None.
TEST_MODE_REPRESENTATIVE_MAX_ATTEMPTSkeeps its name — it is monkeypatched by an existing test.Changed Signature
None.
Changed Behaviour
PYAUTO_TEST_MODE=2— a model whose assertions reject the prior medians no longer raises during the fit; the bypass evaluates the first accepted deterministic draw instead and logs a WARNING naming the rejection and the draw index.PYAUTO_TEST_MODE=3— same point selection, so the stored samples are reconstructible andresult.max_log_likelihood_instanceno longer raisesSamplesException. This mode now instantiates the model once (see above).PYAUTO_TEST_MODE=1— unchanged, including the exact"TEST MODE 1 could not construct a valid representative result after N attempts"message and the per-sample validation.Migration
None required.
Gate
pyauto-heartis not reachable from this environment (pyauto-brain vitals→'pyauto-heart' not found on PATH), so the readiness gate ran in the documented fallback form (PyAutoBrain/skills/WORKFLOW.md): the full library suite as the gate, any failure treated as RED. It came back 2016 passed / 34 skipped / 0 failed. This is not a Heart GREEN verdict — a Heart-reachable environment should confirm before merge.Closes #1519
Generated by the PyAutoLabs agent workflow.
Generated by Claude Code