Sensitivity: Minor plotting fixes

[cvolkcvolk]

Summary

Minor improvements in the sensitivity posterior-marginal plots

Detailed description

• Prior reference instead of the mean: the continuous panels drop the posterior-mean line and instead overlay the uniform prior as a flat reference plus a shaded 5–95% band.
• Shared y-axis across a vector variation's components: share a y-axis so their densities are directly comparable;

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

Replaces the posterior-mean line in the continuous marginal panels with a uniform-prior reference at 1/span plus a shaded 5-95% credible band. The reasoning holds: for a factor swept symmetrically around its nominal value the mean collapses onto the prior mean and hides the signal, whereas reading the posterior against the uniform prior shows concentration directly. I confirmed the prior is BoxUniform over the normalized range (analyzer.py:_normalized_prior), so 1/span is the correct reference density in original units. Rendering-only and well-scoped — just two minor visualization notes below.

Findings

🔵 Improvement: plotting.py:100 — the 5-95% band reuses the KDE fill color, so it mostly blends into the fill (see inline).
🔵 Improvement: plotting.py:96 — KDE boundary leakage interacts with the new 1/span reference at the edges (see inline).

Test Coverage

Rendering-only matplotlib change with no new inference/data logic, and the suite doesn't assert on figure contents — so no new test is warranted here, consistent with the rest of the module.

Verdict

Ship it

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

A focused, well-reasoned change to the sensitivity marginal renderer: it drops the posterior-mean line (uninformative for a symmetric sweep) in favour of overlaying the uniform prior as a flat reference plus a shaded 5–95% credible band, and gives the components of a vector variation a shared y-axis so their densities compare directly. The rationale is sound and the implementation is contained to one pure-plotting file. I verified the prior density (1/span), the percentile band, the KDE no-spread guard, and the shared-axis grouping — including the constant-component edge case, which degrades gracefully.

Findings

🔵 Improvement: isaaclab_arena/analysis/sensitivity/plotting.py:117 — minor baseline inconsistency between the two branches (see inline).

Test Coverage

No tests added, which is reasonable here — this is a visual-rendering change with no extractable numeric contract, and the existing sensitivity tests still exercise the call path. Nothing actionable.

Verdict

Ship it

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

A focused, rendering-only change to the sensitivity posterior-marginal plots: the continuous panels now overlay the uniform prior as a flat reference plus a 5–95% credible band instead of a posterior-mean line, and components of a vector variation share a y-axis. The motivation is sound — a mean is uninformative for a factor swept symmetrically about its nominal value, and reading the posterior against the prior shows concentration directly. No change to inference or the data model. Looks good to ship; one small robustness question below.

Findings

🔵 Improvement: isaaclab_arena/analysis/sensitivity/plotting.py:64 — grouping by stripping [\d+]$ from the name recovers the vector-component relationship from a string convention. It's the only carrier available today (FactorSpec has no structured base-name), so this is reasonable — but a standalone scalar factor whose name legitimately ends in [0] would be folded into a shared-scale group. If factor names are guaranteed to only use the name[i] suffix for true vector components, this is fine as-is; worth a one-line note to that effect.

Test Coverage

Pure rendering change with no behavioral surface to assert on, and the repo only exercises plot_marginals through the manual tests/sensitivity_synthetic.py pipeline script (not a pytest), so no automated test is expected here. The PR notes it was verified by regenerating the report, which is the right check for this kind of change.

Verdict

Ship it

[greptile-apps]

Greptile Summary

This PR improves the sensitivity posterior-marginal plot in two ways: the per-panel mean reference line is replaced by a flat uniform-prior dash and a shaded 5–95% posterior band, and continuous panels that belong to the same vector variation now share a y-axis so their densities are directly comparable.

• Prior reference line: _draw_continuous_marginal now draws axhline(1.0 / span) as the "no-effect" baseline and shades the 5–95% sample percentile range instead of marking the posterior mean.
• Shared y-axis: after all panels are drawn, axes grouped by the vector-component naming convention (name[0], name[1], …) have their y-limits unified to the group maximum, enabling direct density comparison across components.

Confidence Score: 4/5

Safe to merge with the prior-line visibility fix applied; without it, the primary new visual element is silently hidden for any factor whose range spans less than 1 unit or whose posterior collapsed to constant samples.

The prior reference dash is drawn after set_ylim has already frozen the y-ceiling, so it is clipped off-screen for constant-sample panels with narrow factor ranges (span < 1) and for KDE panels whose in-range density peak falls below the prior height. This affects real robotics factors (friction, joint limits, PD gains) that routinely live in sub-unit ranges.

isaaclab_arena/analysis/sensitivity/plotting.py — specifically the ordering of set_ylim and axhline in _draw_continuous_marginal.

Important Files Changed

Filename	Overview
isaaclab_arena/analysis/sensitivity/plotting.py	Adds prior reference line and shared y-axis for vector factor components; the prior axhline is drawn after set_ylim freezes the y-ceiling, making it invisible for constant-sample or small-span panels.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[plot_marginals called] --> B[Create subplot grid]
    B --> C{For each factor}
    C --> D{factor.type == continuous?}
    D -- Yes --> E[_draw_continuous_marginal]
    E --> F[KDE branch: std >= 1e-9?]
    F -- Yes --> G[Plot KDE + fill_between\nset_ylim bottom=0\naxvspan 5-95% band]
    F -- No --> H[axvline at mean\nset_ylim bottom=0]
    G --> I[axhline 1/span prior ref]
    H --> I
    I --> J[set_xlim, legend, grid]
    J --> K[Group ax by variation name]
    D -- No --> L[_draw_categorical_marginal]
    C --> M{More factors?}
    M -- Yes --> C
    M -- No --> N[Shared y-axis loop for grouped axes]
    N --> O[shared_top = max get_ylim\nset_ylim 0 shared_top]
    O --> P[suptitle + tight_layout + save]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[plot_marginals called] --> B[Create subplot grid]
    B --> C{For each factor}
    C --> D{factor.type == continuous?}
    D -- Yes --> E[_draw_continuous_marginal]
    E --> F[KDE branch: std >= 1e-9?]
    F -- Yes --> G[Plot KDE + fill_between\nset_ylim bottom=0\naxvspan 5-95% band]
    F -- No --> H[axvline at mean\nset_ylim bottom=0]
    G --> I[axhline 1/span prior ref]
    H --> I
    I --> J[set_xlim, legend, grid]
    J --> K[Group ax by variation name]
    D -- No --> L[_draw_categorical_marginal]
    C --> M{More factors?}
    M -- Yes --> C
    M -- No --> N[Shared y-axis loop for grouped axes]
    N --> O[shared_top = max get_ylim\nset_ylim 0 shared_top]
    O --> P[suptitle + tight_layout + save]

Loading

_{Reviews (2): Last reviewed commit: "Update isaaclab_arena/analysis/sensitivi..." | Re-trigger Greptile}

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

A small, well-documented plotting change in analysis/sensitivity/plotting.py: the continuous marginals now overlay the uniform prior plus a 5–95% credible band instead of a posterior-mean line, and components of a vector variation share a y-axis for direct comparison. The grouping logic and the shared-y-axis pass are correct, and the change is scoped to a niche analysis renderer with no boundary or default-behavior concerns. One minor maintainability note below.

Findings

🔵 Improvement: isaaclab_arena/analysis/sensitivity/plotting.py:64 — the component grouping reverses the key[i] suffix produced in episode_results_reader.py. That reader carries a TODO(cvolk) to emit semantic component names instead of the positional [i] suffix — when that lands, this regex would silently stop matching and each component would fall back to its own scale with no error. Worth a NOTE here (or in that TODO) flagging that the grouping depends on the [i] format, so the two stay in sync.

Test Coverage

This is a pure-rendering visual change (KDE overlays, axis limits) with no behavioral contract that lends itself to a meaningful assertion, so the lack of a new test is reasonable here.

Verdict

Ship it