Skip to content

docs(search): state the opt-in rule for _test_mode_samples_info - #1449

Merged
Jammy2211 merged 1 commit into
mainfrom
feature/test-mode-samples-info-hook-contract
Aug 4, 2026
Merged

docs(search): state the opt-in rule for _test_mode_samples_info#1449
Jammy2211 merged 1 commit into
mainfrom
feature/test-mode-samples-info-hook-contract

Conversation

@Jammy2211

Copy link
Copy Markdown
Collaborator

Summary

NonLinearSearch._test_mode_samples_info() told subclasses to override it "so
that tutorial scripts and downstream code can access those keys without
KeyError" — which reads as a per-sampler obligation. Nine search modules write
samples_info in their real path (nautilus, dynesty, emcee, zeus, bfgs, drawer,
blackjax nuts, multi_start_gradient, abstract) and exactly one overrides the
hook
, so the other eight look like they are missing one.

They aren't. This PR documents the rule the existing decisions actually follow.

Reachability. No library path reads these diagnostic keys under bypass. The
properties that read them — SamplesMCMC.total_steps (samples/mcmc.py:200),
SamplesNest.number_live_points / total_samples / log_evidence
(samples/nest.py:77-92) — live on Samples subclasses the bypass never
constructs; _fit_bypass_test_mode always builds a SamplesPDF. The only
consumers are workspace scripts reading samples_info[...] directly.

Consumer sweep across all eleven workspace/tutorial repos finds exactly two:

Site Keys Runs bypassed?
autofit_workspace/scripts/searches/mcmc.py:335 ess_min, num_samples, mean_acceptance, n_divergent, n_logl_evals yes — no __Env__, and it is line 2 of that workspace's smoke_tests.txt, so it runs bypassed on every PR
autofit_workspace_test/.../multi_start_gradient_auto_convergence.py:130 total_steps no — declares ENV: real_search jax

The first is why the NUTS override exists (#1260). The second was fixed on the
workspace side (autofit_workspace_test#83 / PR#84, merged f4c45c1). Nautilus,
Dynesty, Emcee and Zeus have no bypassed consumer either — hence no override.
autofit_workspace/scripts/searches/mle.py uses MultiStartAdam but never
touches samples_info, so MultiStartGradient has no bypassed consumer at all.

The rule, now in the docstring: prints → placeholders; asserts → real search.
A tutorial that only displays diagnostics may legitimately run bypassed, so its
search needs the override with honest empties. A script that asserts on them
must not run bypassed at all — it declares ENV: real_search. Adding
placeholders for an asserting reader is worse than the KeyError it replaces,
because the assert then silently passes on a stub value.

API Changes

None. Docstring-only — no signature, behaviour or output change.

Rejected alternatives

  • Add an AbstractMultiStartGradient override for surface consistency with
    NUTS: it serves no existing consumer, and a placeholder total_steps would let
    a future assert total_steps < n_steps silently pass on a stub 0 — the exact
    failure mode avoided in autofit_workspace_test#83.
  • A unit test pinning the bypass keys: a test can pin BlackJAXNUTS's keys
    but cannot express "and the others deliberately have none" without freezing the
    sampler roster.

Test Plan

  • pytest test_autofit/non_linear/search172 passed, 1 skipped
  • Docstring renders and is reachable at runtime
    (NonLinearSearch._test_mode_samples_info.__doc__)
  • Diff is one file, docstring only

Closes #1448

Generated by the PyAutoLabs agent workflow.

The hook's docstring told subclasses to override it 'so that tutorial
scripts and downstream code can access those keys without KeyError',
which reads as a per-sampler obligation — so the fact that only
BlackJAXNUTS overrides it, out of nine searches that write samples_info,
looks like an oversight in the other eight.

It isn't. No library path reads these diagnostic keys under bypass: the
properties that read them (SamplesMCMC.total_steps,
SamplesNest.total_samples, ...) live on Samples subclasses the bypass
never constructs, since _fit_bypass_test_mode always builds a SamplesPDF.
The only consumers are workspace scripts reading samples_info[...]
directly, and across all eleven workspace/tutorial repos there are
exactly two: searches/mcmc.py prints NUTS diagnostics (bypassed on every
PR — no __Env__ declaration, and it is in smoke_tests.txt), which is why
#1260 added the hook and the NUTS override; and the multi-start
auto-convergence assertion script, which asserts on total_steps and was
fixed with ENV: real_search jax instead (autofit_workspace_test#83).

Document the rule those two decisions actually follow — prints ->
placeholders, asserts -> real search — and warn that adding placeholders
for an asserting reader is worse than the KeyError it replaces, since the
assert then silently passes on a stub value.

Docstring only; no behaviour change.

Closes #1448

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Jammy2211 Jammy2211 added the pending-release PR queued for the next release build label Aug 4, 2026
@Jammy2211
Jammy2211 merged commit 8f706c5 into main Aug 4, 2026
3 checks passed
@Jammy2211
Jammy2211 deleted the feature/test-mode-samples-info-hook-contract branch August 4, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pending-release PR queued for the next release build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(search): state the opt-in rule for _test_mode_samples_info

1 participant