fix: warn when pyro posterior params fall back to uniform - #23
Merged
Conversation
_ensure_pyro_posterior_params silently re-initialized 'q_p_ct_raw' to a uniform 1/P simplex whenever the Pyro param store lacked it. That masked failed/missing param-store loads: downstream posterior metrics then ran on an uninformative prior with no signal to the caller (Notion #24). Emit a RuntimeWarning before the fallback, naming the affected metrics and the likely cause (param store not persisted/restored). Behavior is otherwise unchanged; the early-return path when 'q_p_ct_raw' is present stays silent. Adds tests/test_pyro_params.py: warns on empty store (and still populates the param), silent when already present. Tests snapshot/restore the global Pyro param store to avoid cross-test leakage. Closes Notion #24. Co-Authored-By: Claude Opus 4.8 <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
_ensure_pyro_posterior_paramsre-initializedq_p_ct_rawto a uniform1/Psimplex whenever the Pyro param store lacked it — silently. This masked failed or missing param-store loads, so downstream posterior metrics ran on an uninformative prior with no signal to the caller.RuntimeWarningbefore the fallback, naming the affected metrics (joint_distribution_posterior, phenotypic/clonotypic entropy, mutual information) and the likely cause (param store not persisted/restored).q_p_ct_rawis already present) stays silent — no behavior change on the happy path.Why
The fallback originally existed because PyTorch >= 2.6
weights_only=Truemadepyro.get_param_store().load()throw silently; the caught exception left metrics running on a uniform prior with no error. With the torch>=2.6 load fixed, this fallback should now only fire on a genuinely empty/missing store — and a re-initialized (uniform) posterior is almost never what the caller wanted, so it should be loud.Tests
NUMBA_CACHE_DIR=... MPLCONFIGDIR=... pytest tests/test_pyro_params.py -v-> 2 passedNew
tests/test_pyro_params.py: warns on empty store (and still populates the param so callers can proceed), silent when already present. Both snapshot/restore the global Pyro param store to avoid cross-test leakage.Closes Notion #24.
🤖 Generated with Claude Code