Sensitivity: interactive Streamlit explorer (draft, stacked)#827
Conversation
…itioning - app.py: fit-once, re-sample-on-change viewer; sidebar outcome toggle and a what-if panel that pins or averages factors and reads the conditioned marginal, with a live slice count - marginals.condition_mask: slice posterior draws to a pinned region (pure, testable) - plotting.plot_marginal / plot_joint: single-panel renderers the app draws through - Streamlit is already a dev dependency; run via streamlit run ... -- --episode_results <path> Signed-off-by: Clemens Volk <cvolk@nvidia.com>
![arena-review-bot[bot]](https://avatars.githubusercontent.com/u/195794629?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Isaac Lab-Arena Review Bot
Summary
Adds a Streamlit explorer (app.py) over the amortized sensitivity posterior, plus a pure condition_mask slicer in marginals.py and single-panel plot_marginal / plot_joint renderers in plotting.py. The core logic is clean — conditioning is a pure mask, seeding flows through torch.manual_seed for reproducibility, and the renderers reuse the existing draw helpers. The main thing worth resolving before this leaves draft is that the PR ships more surface than the app currently uses, and the module docstring describes views the app doesn't render yet.
Findings
🟡 Warning: plotting.py:217 — plot_joint is fully implemented and exported but unused — app.py imports only plot_marginal, and there's no other caller in the repo. It looks staged for a factor-pair selector that isn't wired up yet. Could we defer it to the MR that actually adds the joint view, so we don't carry an untested ~50-line public renderer in the meantime? (Same question, lighter, for the output_path arg on plot_marginal — the only caller never passes it; though it does mirror plot_marginals for report symmetry, so leaving it is defensible.)
🔵 Improvement: app.py:10 — the module docstring says every control change "redraws the importance ranking, the per-factor marginals, and a chosen pairwise joint," but main() only renders the conditioning panel's single marginal — no importance chart, no full-marginals grid, no joint. Could we trim the docstring to what the app does today (and add the rest when it's wired)?
🔵 Improvement: plotting.py:205 — the observation_label = ", ".join(...) block is now duplicated in five functions (plot_marginals, plot_marginal, plot_joint, plot_corner, plot_importance). Worth pulling into a small _observation_label(dataset, observation) -> str helper?
Test Coverage
condition_mask is the new pure, testable piece (the PR description calls it out as such) and has no test — though the whole sensitivity/ module currently has no unit tests, so this isn't a regression in coverage. The renderers and the Streamlit shell are reasonably left untested. If you add one thing, a small condition_mask test (a couple of pinned windows / categorical codes, asserting the surviving rows) would lock in the slicing contract cheaply.
Verdict
Minor fixes needed — and appropriate to leave as draft until the importance/joint views are wired and the docstring matches.
| return figure | ||
|
|
||
|
|
||
| def plot_joint( |
There was a problem hiding this comment.
🟡 plot_joint is implemented and exported but unused — app.py imports only plot_marginal, and grep finds no other caller. It reads as staged for a pairwise-joint selector that isn't wired up yet. Could we defer it to the MR that adds the joint view rather than carry an untested public renderer here? (If you'd rather keep it, wiring a factor-pair selectbox + st.pyplot(plot_joint(...)) into the app would justify it.)
|
|
||
| The estimator is amortized, so re-conditioning it on a new outcome is a cheap re-sample with no | ||
| retraining. This app exposes that: the posterior is fit once (cached), and every control change | ||
| re-samples it and redraws the importance ranking, the per-factor marginals, and a chosen pairwise |
There was a problem hiding this comment.
🔵 This says the app redraws "the importance ranking, the per-factor marginals, and a chosen pairwise joint," but main() only renders the conditioning panel's single marginal — none of those three. Could we trim the docstring to what the app does today and re-add the rest as the views land?
| else: | ||
| _draw_categorical_marginal(ax, factor, factor_samples) | ||
|
|
||
| observation_label = ", ".join( |
There was a problem hiding this comment.
🔵 This observation_label join is now duplicated across five functions in this file. Worth extracting a small _observation_label(dataset, observation) -> str helper and calling it here and in plot_joint / plot_marginals / plot_corner / plot_importance?
1 participant
Summary
Interactive sensitivity explorer
Detailed description