|
| 1 | +## test-mode-bypass-assertion-ties |
| 2 | +- issue: https://github.com/PyAutoLabs/PyAutoFit/issues/1519 (closed) |
| 3 | +- completed: 2026-08-24 |
| 4 | +- library-pr: https://github.com/PyAutoLabs/PyAutoFit/pull/1520 (merged 438f56fac) |
| 5 | +- summary: The PYAUTO_TEST_MODE=2/3 bypass evaluated the model at the prior |
| 6 | + medians, so a model with identical priors plus an ordering assertion (the |
| 7 | + exchange-degeneracy idiom, e.g. PyAutoCTI trap models) tied exactly there and |
| 8 | + check_assertions hard-failed the run. The bypass now picks its point through a |
| 9 | + shared `_test_mode_valid_parameter_vector` — prior medians first, then |
| 10 | + `default_rng(seed=0)` prior draws, each candidate validated — so the vector it |
| 11 | + evaluates AND stores satisfies the assertions. |
| 12 | +- key-finding: **the prompt's own suggested fix would not have worked.** The |
| 13 | + 2026-08-09 note had narrowed this to a one-liner (move `instance_from_vector` |
| 14 | + inside the existing try, or pass `ignore_assertions=True`). Reading main |
| 15 | + d3625a8 found THREE sites, and both one-liners fix only the first: |
| 16 | + 1. `abstract_search.py:1007` — instantiation outside the FitException guard; |
| 17 | + 2. `_build_fake_samples:1112` — the perturbed samples are the median vector |
| 18 | + scaled UNIFORMLY (1.001/0.999/1.002), and a uniform scale preserves an |
| 19 | + ordering tie, so every stored sample fails the same assertion; |
| 20 | + 3. `SamplesSummary.max_log_likelihood` (`interface.py:122`) is `@to_instance()` |
| 21 | + with `recover="raise"` — it extends SamplesInterface directly so it does NOT |
| 22 | + inherit Samples' next-valid recovery — so `result.max_log_likelihood_instance` |
| 23 | + raises SamplesException, and `Result.instance` catches only AttributeError. |
| 24 | +- key-finding: **TEST_MODE=3 was broken too and nobody had noticed.** It never |
| 25 | + calls instance_from_vector in the bypass, so it survived the fit and died at |
| 26 | + the first `result.max_log_likelihood_instance`. Confirmed by reproduction, not |
| 27 | + just by reading. Fixing the stored vector fixes modes 2 and 3 together. |
| 28 | +- trap: do NOT re-run a bypass reproduction without clearing `output/` first. A |
| 29 | + bypassed fit calls `paths.completed()`, so a second run with the same |
| 30 | + unique_tag takes `result_via_completed_fit` and replays the OLD (broken) |
| 31 | + samples — which reads exactly like "the fix didn't work". Cost one false |
| 32 | + negative during verification. |
| 33 | +- trap: `af.m.MockAnalysis` maps its likelihood over the model and returns a |
| 34 | + LIST for an `af.Collection`, which the bypass's `float()` rejects. Regression |
| 35 | + tests needed a small float-returning analysis instead. |
| 36 | +- behaviour-change: mode 3 now instantiates the model once (previously zero |
| 37 | + times). A model whose constructor raises a non-FitException at the medians now |
| 38 | + fails at fit time rather than result time — same failure, surfaced earlier. |
| 39 | + Flagged in the PR body for downstream repos. |
| 40 | +- verification: reproduced on clean main first (mode 2 raised FitException in the |
| 41 | + fit; mode 3 raised SamplesException at result access), then both modes complete |
| 42 | + and select the identical vector after. All 5 new tests in |
| 43 | + `TestBypassToleratesAssertionTies` fail against the un-patched source. Full |
| 44 | + suite 2016 passed / 34 skipped / 0 failed; CI green on all three legs |
| 45 | + (unittest 3.12, unittest 3.13, unittest-nojax) plus Docs. |
| 46 | +- gate-caveat: shipped from a web-github session where `pyauto-heart` is |
| 47 | + unreachable, so the readiness gate ran in the WORKFLOW.md fallback form (full |
| 48 | + library suite as the gate). No Heart verdict was recorded for this task; CI |
| 49 | + green at merge is the stronger confirmation that stands in its place. The |
| 50 | + workspace-impact grep was likewise not run (workspace clones absent) — API |
| 51 | + Changes are "none, internal", so option (iii) was inferred, not measured. |
| 52 | +- follow-up: `autocti_workspace` documents this artifact in its AGENTS.md as a |
| 53 | + workaround. Delete that note now the fix has shipped — the testmode-env-drift |
| 54 | + precedent ("delete the trap, don't document it"). Separate repo, separate task. |
| 55 | +- follow-up: re-enable autocti_workspace smoke coverage of the |
| 56 | + `modeling/start_here.py`-class scripts (CTI epic Phase 5) that this unblocks. |
| 57 | +- environment: web-github; no worktree was ever created, so there is none to |
| 58 | + remove. PyAutoFit was worked in a session clone at /home/user/pyautofit. |
| 59 | + |
| 60 | +## Original prompt |
| 61 | + |
| 62 | +# TEST_MODE bypass crashes on ordered-parameter assertion ties |
| 63 | + |
| 64 | +Type: bug |
| 65 | +Target: PyAutoFit |
| 66 | +Repos: |
| 67 | +- @PyAutoFit |
| 68 | +Difficulty: small |
| 69 | +Autonomy: supervised |
| 70 | +Priority: normal |
| 71 | +Status: formalised — STILL REPRODUCES; see the 2026-08-09 note before grading this against main |
| 72 | +Filed: 2026-07-17 (backfilled from git) |
| 73 | +Issued: 2026-08-24 |
| 74 | + |
| 75 | +## 2026-08-09 — do NOT mistake the adjacent FitException catch for this fix |
| 76 | + |
| 77 | +Checked by the draft/ sweep against PyAutoFit main (`3b960609`). The bypass path |
| 78 | +in `abstract_search.py` **now catches `exc.FitException`** and continues with the |
| 79 | +`-1e99` sentinel, logging "TEST MODE 2: likelihood verification raised |
| 80 | +FitException … treating as a resample-rejected instance". That reads exactly like |
| 81 | +this prompt's suggested fix. **It is not.** The bug below still reproduces. |
| 82 | + |
| 83 | +The catch wraps only the likelihood call. The model instantiation is on the line |
| 84 | +*before* the `try`: |
| 85 | + |
| 86 | +```python |
| 87 | +if call_likelihood: |
| 88 | + instance = model.instance_from_vector(vector=parameter_vector) # <-- outside |
| 89 | + try: |
| 90 | + log_likelihood = float(analysis.log_likelihood_function(instance)) |
| 91 | + except exc.FitException as e: |
| 92 | + ... |
| 93 | +``` |
| 94 | + |
| 95 | +and `instance_from_vector` → `instance_for_arguments` → `check_assertions` |
| 96 | +(`autofit/mapper/prior_model/abstract.py:193`) is precisely what raises |
| 97 | +`exc.FitException("N assertions failed!")` when an ordering assertion ties at the |
| 98 | +prior medians. `ignore_assertions` defaults to `False` and the bypass does not |
| 99 | +pass it. So the assertion exception escapes the guard entirely and still |
| 100 | +hard-fails the run. |
| 101 | + |
| 102 | +The upside: the fix is now a one-liner rather than the "catch and retry with a |
| 103 | +perturbation" design sketched below. Two options, both cheap and both |
| 104 | +deterministic: |
| 105 | + |
| 106 | +- move the `instance_from_vector` call inside the existing `try` — the sentinel |
| 107 | + path already does the right thing for a rejected instance; or |
| 108 | +- pass `ignore_assertions=True` at the bypass instantiation, on the grounds that |
| 109 | + a verification eval at the medians is not a sampled point and assertions exist |
| 110 | + to steer sampling. |
| 111 | + |
| 112 | +The second is probably the better semantics (a tied median is not a pathological |
| 113 | +model), but it changes what the verification eval attests to — pick deliberately. |
| 114 | +Prefer either over adding perturbation logic. |
| 115 | + |
| 116 | +`Difficulty:` stays small. The § Blocks note below still holds. |
| 117 | + |
| 118 | +--- |
| 119 | + |
| 120 | +Found during the CTI resurrection epic (Phase 4, 2026-07-17). `PYAUTO_TEST_MODE=2/3` |
| 121 | +bypass evaluates the model at the **prior medians**. A model whose components have |
| 122 | +identical priors plus an ordering assertion (the standard idiom for breaking |
| 123 | +exchange degeneracy, e.g. PyAutoCTI trap models with |
| 124 | +`model.add_assertion(trap_0.release_timescale < trap_1.release_timescale)`) |
| 125 | +ties exactly at the medians, so the bypass evaluation raises |
| 126 | +`autofit.exc.FitException: GreaterThanLessThanAssertion` and the script crashes. |
| 127 | + |
| 128 | +Real samplers resample assertion-failing points gracefully — this is purely a |
| 129 | +bypass-path artifact, and it makes every ordered-trap CTI workspace script |
| 130 | +un-smokeable at TEST_MODE=2 (reproduced with a bare |
| 131 | +`model.instance_from_prior_medians()`; TEST_MODE=1 passes). |
| 132 | + |
| 133 | +Suggested fix: at the bypass evaluation, catch `FitException` from assertions |
| 134 | +and retry with a small deterministic perturbation of the unit-cube point (or a |
| 135 | +seeded random draw), mirroring what a real sampler does. Keep it deterministic |
| 136 | +so smoke runs stay reproducible. |
| 137 | + |
| 138 | +Blocks: autocti_workspace smoke coverage of `modeling/start_here.py`-class |
| 139 | +scripts (CTI epic Phase 5); the workspace documents the artifact in its |
| 140 | +AGENTS.md meanwhile. |
0 commit comments