docs(search): state the opt-in rule for _test_mode_samples_info - #1449
Merged
Conversation
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>
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
NonLinearSearch._test_mode_samples_info()told subclasses to override it "sothat tutorial scripts and downstream code can access those keys without
KeyError" — which reads as a per-sampler obligation. Nine search modules writesamples_infoin 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 onSamplessubclasses the bypass neverconstructs;
_fit_bypass_test_modealways builds aSamplesPDF. The onlyconsumers are workspace scripts reading
samples_info[...]directly.Consumer sweep across all eleven workspace/tutorial repos finds exactly two:
autofit_workspace/scripts/searches/mcmc.py:335ess_min,num_samples,mean_acceptance,n_divergent,n_logl_evals__Env__, and it is line 2 of that workspace'ssmoke_tests.txt, so it runs bypassed on every PRautofit_workspace_test/.../multi_start_gradient_auto_convergence.py:130total_stepsENV: real_search jaxThe 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.pyusesMultiStartAdambut nevertouches
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. Addingplaceholders for an asserting reader is worse than the
KeyErrorit replaces,because the assert then silently passes on a stub value.
API Changes
None. Docstring-only — no signature, behaviour or output change.
Rejected alternatives
AbstractMultiStartGradientoverride for surface consistency withNUTS: it serves no existing consumer, and a placeholder
total_stepswould leta future
assert total_steps < n_stepssilently pass on a stub0— the exactfailure mode avoided in autofit_workspace_test#83.
BlackJAXNUTS's keysbut cannot express "and the others deliberately have none" without freezing the
sampler roster.
Test Plan
pytest test_autofit/non_linear/search— 172 passed, 1 skipped(
NonLinearSearch._test_mode_samples_info.__doc__)Closes #1448
Generated by the PyAutoLabs agent workflow.