diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 8670e543..80ec7019 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -27,6 +27,12 @@ targeted `Edit` patches. Do not suppress clippy warnings with `#[allow(...)]`. - **GPG-signed commits**: always use `git commit -S`. The GPG agent is working. - **Atomic commits**: commit early and often. Huge monolithic commits make it hard to catch off-rail behaviour. +- **PR descriptions are concise and professional**: a short summary of what + changed and why, the validation performed, and any risk or follow-up — + a few sentences or bullets total. Do NOT inventory individual edits (the + diff shows those), narrate review rounds, or paste investigation memos. + Long-form audit/investigation material goes in a PR **comment** or the + commit messages, never the description. - **No temp file litter**: clean up all one-off scripts immediately. - **Do not touch `.claude/worktrees/`**: this directory is managed by Claude Code for isolated worktree sessions. Never delete, modify, or @@ -158,12 +164,13 @@ when working single-threaded on one feature branch. **Anything else does not belong in `docs/`.** In particular: - **Audit memos** (impact analysis of a bug fix, which research scripts - were affected, etc.) — belong in the PR body. When the PR merges, - the memo is preserved in the commit + PR history where it is actually - searchable. Do not create `docs/audit/` or similar. + were affected, etc.) — belong in a PR **comment**, never the PR + description (see Commit Style). When the PR merges, the memo is + preserved in the PR history where it is actually searchable. Do not + create `docs/audit/` or similar. - **Investigation / ablation / debugging notes** (step-by-step inquiry into a performance question, correctness hypothesis, etc.) — belong in - `.research/` (gitignored; session-private) or in the PR body if the + `.research/` (gitignored; session-private) or in a PR comment if the investigation directly motivates a concrete code change. - **Design specs / requirements drafts** — belong inline with the code they specify (rustdoc on the struct / module they describe) or in @@ -173,7 +180,7 @@ when working single-threaded on one feature branch. If I feel compelled to commit a memo under `docs/`, I stop and check: (a) is this user-facing reference that someone reading the project cold will want to read? If yes, it fits one of the subtrees above. -If no, it belongs in `.research/` or the PR body. **When in doubt, +If no, it belongs in `.research/` or a PR comment. **When in doubt, ask the user before creating the file** — do not invent a new `docs//` directory to make the artifact commit-able. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37d9c45f..8cdf9b8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,9 +19,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 - run: cargo check --workspace --exclude nereids-python # Compile-check workspace examples so docs snippets cannot drift @@ -50,9 +50,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml components: rustfmt - run: cargo fmt --all -- --check @@ -70,9 +70,9 @@ jobs: os: [ubuntu-latest, macos-latest] steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml components: clippy - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 - run: cargo clippy --workspace --exclude nereids-python --all-targets -- -D warnings @@ -95,9 +95,9 @@ jobs: os: [macos-latest, windows-latest] steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 - name: Build run: cargo build --workspace --exclude nereids-python @@ -111,9 +111,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 - name: Build docs run: cargo doc --workspace --no-deps --exclude nereids-python @@ -141,9 +141,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 - name: Install pixi uses: prefix-dev/setup-pixi@a0af7a228712d6121d37aba47adf55c1332c9c2e # v0.9.4 @@ -159,12 +159,16 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml components: llvm-tools-preview - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 - - uses: taiki-e/install-action@50570a4fd0cb7e583e6a512c02bffb9ade28d868 # cargo-llvm-cov + # Pin release-tag SHAs only (like security.yml), never taiki-e's + # per-tool tags (e.g. `cargo-llvm-cov`): those are orphan commits on + # no branch, and Dependabot's github_actions updater hard-errors on + # them ("no such commit"), killing the whole update run. + - uses: taiki-e/install-action@c7eb1735f09259a5035e8e5d44b1406b1cddc0fb # v2.83.0 with: tool: cargo-llvm-cov - name: Generate coverage diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index f4caee87..50a8c941 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -47,9 +47,9 @@ jobs: - name: Check curated Python API page is in sync with .pyi stubs run: python scripts/check_python_api_drift.py - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 04d930ea..027c7482 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -161,9 +161,9 @@ jobs: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 @@ -270,9 +270,9 @@ jobs: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master with: - toolchain: stable + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml - name: Publish crates (dependency order) env: diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index e76bf738..4f3cf6da 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -20,8 +20,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable - - uses: taiki-e/install-action@4684b8405694ae9dd42c9f39ba901a70ae83f4a3 # v2.82.9 + - uses: dtolnay/rust-toolchain@fa04a1451ff1842e2626ccb99004d0195b455a88 # master + with: + toolchain: "1.96.1" # keep in sync with rust-toolchain.toml + - uses: taiki-e/install-action@c7eb1735f09259a5035e8e5d44b1406b1cddc0fb # v2.83.0 with: tool: cargo-audit # Fails on unignored vulnerabilities. Accepted-risk advisories (with a diff --git a/.github/workflows/wheel-policy.yml b/.github/workflows/wheel-policy.yml index 5da40349..1265c1a0 100644 --- a/.github/workflows/wheel-policy.yml +++ b/.github/workflows/wheel-policy.yml @@ -23,6 +23,10 @@ on: - '.github/workflows/publish.yml' - '.github/workflows/wheel-policy.yml' - 'scripts/check_wheel_policy.sh' + # maturin-action resolves the in-container compiler from the repo + # root rust-toolchain.toml, so a pin-only bump changes the release + # wheel compiler and MUST schedule this gate. + - 'rust-toolchain.toml' permissions: contents: read diff --git a/.gitignore b/.gitignore index f6d430f8..13c69d1d 100644 --- a/.gitignore +++ b/.gitignore @@ -58,3 +58,6 @@ tmp/ # do not track manuscript folders .manuscripts/ + +# Session-local agent-harness state (task contracts, verify gates) +.harness/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6acffc7d..8ffdf087 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,7 +22,7 @@ Open a GitHub issue with the `enhancement` label. Include: ### Prerequisites -- [Rust](https://www.rust-lang.org/tools/install) (stable toolchain) +- [Rust](https://www.rust-lang.org/tools/install) (rustup; the exact version is pinned by rust-toolchain.toml) - [pixi](https://pixi.sh/) (Python environment manager) - cmake (for HDF5 builds) diff --git a/apps/gui/src/file_dialog.rs b/apps/gui/src/file_dialog.rs index d3b132ab..0f4a7bcc 100644 --- a/apps/gui/src/file_dialog.rs +++ b/apps/gui/src/file_dialog.rs @@ -502,7 +502,7 @@ impl FileDialogs { egui::Window::new("native_dialog_pending") .title_bar(false) .resizable(false) - .anchor(egui::Align2::CENTER_TOP, [0.0, 8.0]) + .anchor(egui::Align2::CENTER_TOP, [0.0_f32, 8.0_f32]) .show(ctx, |ui| { ui.horizontal(|ui| { ui.spinner(); diff --git a/apps/gui/src/guided/analyze.rs b/apps/gui/src/guided/analyze.rs index 551f2959..81d4e4c0 100644 --- a/apps/gui/src/guided/analyze.rs +++ b/apps/gui/src/guided/analyze.rs @@ -764,7 +764,7 @@ fn fit_feedback_panel(ui: &mut egui::Ui, state: &AppState) { }; egui::Frame::default() .fill(bg_color) - .stroke(egui::Stroke::new(1.5, border_color)) + .stroke(egui::Stroke::new(1.5_f32, border_color)) .corner_radius(4.0) .inner_margin(6.0) .show(ui, |ui| { @@ -999,7 +999,7 @@ fn image_color_bar( painter.rect_stroke( response.rect, 0.0, - egui::Stroke::new(1.0, ui.visuals().widgets.noninteractive.bg_stroke.color), + egui::Stroke::new(1.0_f32, ui.visuals().widgets.noninteractive.bg_stroke.color), egui::StrokeKind::Inside, ); ui.horizontal(|ui| { @@ -1446,7 +1446,7 @@ fn spectrum_panel(ui: &mut egui::Ui, state: &mut AppState) { .collect() }; let measured_points = Points::new("Measured", measured_points) - .radius(1.4) + .radius(1.4_f32) .color(crate::theme::semantic::RED); // OB reference line (counts mode only): the c·OB curve the KL @@ -1721,7 +1721,7 @@ fn draw_aligned_tick_row( painter.rect_stroke( track_rect, 2.0, - egui::Stroke::new(1.0, ui.visuals().widgets.noninteractive.bg_stroke.color), + egui::Stroke::new(1.0_f32, ui.visuals().widgets.noninteractive.bg_stroke.color), egui::StrokeKind::Inside, ); @@ -1761,7 +1761,7 @@ fn draw_aligned_tick_row( egui::pos2(x_pos, track_rect.top()), egui::pos2(x_pos, track_rect.bottom()), ], - egui::Stroke::new(1.0, row.color), + egui::Stroke::new(1.0_f32, row.color), ); } } @@ -1869,7 +1869,7 @@ fn draw_dragged_resonance_ruler( )); painter.line_segment( [egui::pos2(x, frame.top()), egui::pos2(x, frame.bottom())], - egui::Stroke::new(2.0, ruler.color), + egui::Stroke::new(2.0_f32, ruler.color), ); painter.text( egui::pos2(x + 6.0, pointer_y), diff --git a/apps/gui/src/guided/detectability.rs b/apps/gui/src/guided/detectability.rs index c9e01df1..2b7ff9b1 100644 --- a/apps/gui/src/guided/detectability.rs +++ b/apps/gui/src/guided/detectability.rs @@ -576,7 +576,7 @@ pub(crate) fn detect_results_panel(ui: &mut egui::Ui, state: &AppState) { .map(|i| [report.energies[i], report.delta_t_spectrum[i]]) .collect(); let color = design::isotope_dot_color(name); - plot_ui.line(Line::new(name.as_str(), points).color(color).width(1.5)); + plot_ui.line(Line::new(name.as_str(), points).color(color).width(1.5_f32)); } }); }); diff --git a/apps/gui/src/guided/forward_model.rs b/apps/gui/src/guided/forward_model.rs index d08f61ca..2a01f47a 100644 --- a/apps/gui/src/guided/forward_model.rs +++ b/apps/gui/src/guided/forward_model.rs @@ -400,7 +400,7 @@ pub(crate) fn fm_spectrum_panel(ui: &mut egui::Ui, state: &mut AppState) { .filter(|&i| x_values[i].is_finite()) .map(|i| [x_values[i], combined[i]]) .collect(); - plot_ui.line(Line::new("Combined T(E)", points).width(2.0)); + plot_ui.line(Line::new("Combined T(E)", points).width(2.0_f32)); } // Per-isotope contribution lines (dashed, colored by isotope hash) diff --git a/apps/gui/src/guided/landing.rs b/apps/gui/src/guided/landing.rs index 1bf41c72..26796465 100644 --- a/apps/gui/src/guided/landing.rs +++ b/apps/gui/src/guided/landing.rs @@ -115,7 +115,7 @@ pub fn landing_step(ui: &mut egui::Ui, state: &mut AppState) { .fill(tc.bg2) .corner_radius(CornerRadius::same(8)) .inner_margin(Margin::symmetric(16, 10)) - .stroke(Stroke::new(1.0, tc.border)) + .stroke(Stroke::new(1.0_f32, tc.border)) .show(ui, |ui| { ui.horizontal(|ui| { ui.vertical(|ui| { @@ -167,7 +167,7 @@ fn landing_card( .fill(tc.bg2) .corner_radius(CornerRadius::same(12)) .inner_margin(Margin::same(20)) - .stroke(Stroke::new(1.0, tc.border)) + .stroke(Stroke::new(1.0_f32, tc.border)) .shadow(Shadow { offset: [0, 1], blur: 3, diff --git a/apps/gui/src/guided/result_widgets.rs b/apps/gui/src/guided/result_widgets.rs index 7447e49e..77b3f879 100644 --- a/apps/gui/src/guided/result_widgets.rs +++ b/apps/gui/src/guided/result_widgets.rs @@ -267,7 +267,7 @@ pub fn draw_colorbar(ui: &mut egui::Ui, data: &ndarray::Array2, colormap: C painter.rect_stroke( bar_rect, 0.0, - egui::Stroke::new(1.0, egui::Color32::GRAY), + egui::Stroke::new(1.0_f32, egui::Color32::GRAY), egui::StrokeKind::Outside, ); diff --git a/apps/gui/src/guided/sidebar.rs b/apps/gui/src/guided/sidebar.rs index 977f2070..d3ce373f 100644 --- a/apps/gui/src/guided/sidebar.rs +++ b/apps/gui/src/guided/sidebar.rs @@ -26,7 +26,7 @@ pub fn guided_sidebar(ui: &mut egui::Ui, state: &mut AppState) { egui::Frame::NONE .fill(colors.bg) .inner_margin(margin) - .stroke(Stroke::new(1.0, colors.border)), + .stroke(Stroke::new(1.0_f32, colors.border)), ) .show_inside(ui, |ui| { if state.sidebar_collapsed || force_compact { @@ -268,7 +268,7 @@ fn collapsed_step_badge( ); } else { ui.painter() - .circle_stroke(center, 10.0, Stroke::new(1.0, colors.border)); + .circle_stroke(center, 10.0, Stroke::new(1.0_f32, colors.border)); ui.painter().text( center, egui::Align2::CENTER_CENTER, @@ -466,7 +466,7 @@ fn pipeline_step_row( } else { // Pending: outlined icon, not a filled status dot. ui.painter() - .circle_stroke(center, 10.0, Stroke::new(1.0, colors.border)); + .circle_stroke(center, 10.0, Stroke::new(1.0_f32, colors.border)); ui.painter().text( center, egui::Align2::CENTER_CENTER, diff --git a/apps/gui/src/guided/wizard.rs b/apps/gui/src/guided/wizard.rs index 68f1363c..b775c4b5 100644 --- a/apps/gui/src/guided/wizard.rs +++ b/apps/gui/src/guided/wizard.rs @@ -46,7 +46,7 @@ fn breadcrumb_bar(ui: &mut egui::Ui, state: &mut AppState) { .stroke(if is_active || is_past { Stroke::NONE } else { - Stroke::new(1.0, tc.border) + Stroke::new(1.0_f32, tc.border) }) .show(ui, |ui| { ui.label(RichText::new(*label).size(11.0).strong().color(text_color)); @@ -250,7 +250,7 @@ fn wizard_confirm(ui: &mut egui::Ui, state: &mut AppState) { .fill(tc.bg2) .corner_radius(CornerRadius::same(8)) .inner_margin(Margin::same(16)) - .stroke(Stroke::new(1.0, tc.accent)) + .stroke(Stroke::new(1.0_f32, tc.accent)) .show(ui, |ui| { ui.set_min_width(full_w - 38.0); ui.label( @@ -331,9 +331,9 @@ fn wizard_option_card( let tc = ThemeColors::from_ctx(ui.ctx()); let fill = if selected { tc.bg3 } else { tc.bg2 }; let border = if selected { - Stroke::new(2.0, tc.accent) + Stroke::new(2.0_f32, tc.accent) } else { - Stroke::new(1.0, tc.border) + Stroke::new(1.0_f32, tc.border) }; let full_w = ui.available_width(); diff --git a/apps/gui/src/studio/mod.rs b/apps/gui/src/studio/mod.rs index 361d7292..7bd566ad 100644 --- a/apps/gui/src/studio/mod.rs +++ b/apps/gui/src/studio/mod.rs @@ -614,7 +614,7 @@ fn bottom_dock(ui: &mut egui::Ui, state: &mut AppState) { egui::Frame::NONE .fill(colors.bg) .inner_margin(egui::Margin::symmetric(12, 8)) - .stroke(egui::Stroke::new(1.0, colors.border)), + .stroke(egui::Stroke::new(1.0_f32, colors.border)), ) .show_inside(ui, |ui| { let labels = &["Isotopes", "Residuals", "Provenance", "Export"]; @@ -1113,7 +1113,7 @@ fn parameter_sidebar(ui: &mut egui::Ui, state: &mut AppState) { egui::Frame::NONE .fill(colors.bg) .inner_margin(egui::Margin::symmetric(10, 8)) - .stroke(egui::Stroke::new(1.0, colors.border)), + .stroke(egui::Stroke::new(1.0_f32, colors.border)), ) .show_inside(ui, |ui| { egui::ScrollArea::vertical().show(ui, |ui| { diff --git a/apps/gui/src/widgets/design.rs b/apps/gui/src/widgets/design.rs index 2cf906e1..255e575b 100644 --- a/apps/gui/src/widgets/design.rs +++ b/apps/gui/src/widgets/design.rs @@ -48,7 +48,7 @@ pub fn card(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui)) { let shadow_alpha = if ui.style().visuals.dark_mode { 76 } else { 20 }; egui::Frame::NONE .fill(tc.bg2) - .stroke(Stroke::new(1.0, tc.border)) + .stroke(Stroke::new(1.0_f32, tc.border)) .corner_radius(CornerRadius::same(10)) .inner_margin(Margin::same(16)) .shadow(Shadow { @@ -125,7 +125,7 @@ pub fn underline_tabs(ui: &mut Ui, labels: &[&str], selected: &mut usize) -> boo egui::pos2(row_rect.left(), baseline), egui::pos2(row_rect.right(), baseline), ], - Stroke::new(1.0, tc.border), + Stroke::new(1.0_f32, tc.border), ); // 2px accent underline on active tab @@ -135,7 +135,7 @@ pub fn underline_tabs(ui: &mut Ui, labels: &[&str], selected: &mut usize) -> boo egui::pos2(rect.left(), baseline), egui::pos2(rect.right(), baseline), ], - Stroke::new(2.0, tc.accent), + Stroke::new(2.0_f32, tc.accent), ); } @@ -201,7 +201,7 @@ pub fn btn_icon(ui: &mut Ui, label: &str, active: bool) -> Response { let (fill, text_color, stroke) = if active { (tc.accent, Color32::WHITE, Stroke::NONE) } else { - (tc.bg2, tc.fg2, Stroke::new(1.0, tc.border)) + (tc.bg2, tc.fg2, Stroke::new(1.0_f32, tc.border)) }; ui.add( egui::Button::new(RichText::new(label).size(11.0).strong().color(text_color)) @@ -231,7 +231,7 @@ pub fn drop_zone(ui: &mut Ui, loaded: bool, label: &str, hint: &str) -> Response let (border_color, border_width, fill) = if loaded { ( semantic::GREEN, - 2.0, + 2.0_f32, Color32::from_rgba_unmultiplied( semantic::GREEN.r(), semantic::GREEN.g(), @@ -240,7 +240,7 @@ pub fn drop_zone(ui: &mut Ui, loaded: bool, label: &str, hint: &str) -> Response ), ) } else { - (tc.fg3, 1.5, Color32::TRANSPARENT) + (tc.fg3, 1.5_f32, Color32::TRANSPARENT) }; // Background + border @@ -358,7 +358,7 @@ pub fn isotope_chip( let fill = if enabled { tc.bg3 } else { tc.bg2 }; egui::Frame::NONE .fill(fill) - .stroke(Stroke::new(1.0, tc.border)) + .stroke(Stroke::new(1.0_f32, tc.border)) .corner_radius(CornerRadius::same(12)) .inner_margin(Margin::symmetric(8, 4)) .show(ui, |ui| { @@ -430,7 +430,7 @@ pub fn group_chip( let fill = if enabled { tc.bg3 } else { tc.bg2 }; egui::Frame::NONE .fill(fill) - .stroke(Stroke::new(1.0, tc.border)) + .stroke(Stroke::new(1.0_f32, tc.border)) .corner_radius(CornerRadius::same(12)) .inner_margin(Margin::symmetric(8, 4)) .show(ui, |ui| { @@ -993,7 +993,7 @@ pub(crate) fn draw_resonance_dips( plot_ui.vline( VLine::new("", res.energy) .color(RESONANCE_DIP_COLOR) - .width(0.5), + .width(0.5_f32), ); } } @@ -1155,7 +1155,7 @@ pub(crate) fn build_fit_line(p: &FitLineParams<'_>) -> Option> { .collect(); Some( Line::new("Fit", fit_points) - .width(1.25) + .width(1.25_f32) .color(egui::Color32::from_rgba_unmultiplied(0, 122, 255, 170)), ) } diff --git a/apps/gui/src/widgets/image_view.rs b/apps/gui/src/widgets/image_view.rs index da4905ef..f096bcd8 100644 --- a/apps/gui/src/widgets/image_view.rs +++ b/apps/gui/src/widgets/image_view.rs @@ -588,7 +588,7 @@ fn draw_roi_draft_overlay( roi_rect, 0.0, egui::Stroke::new( - 1.5, + 1.5_f32, egui::Color32::from_rgba_unmultiplied(255, 255, 255, 180), ), egui::StrokeKind::Outside, @@ -681,7 +681,7 @@ fn draw_roi_overlay( painter.rect_stroke( roi_rect, 0.0, - egui::Stroke::new(1.5, egui::Color32::from_rgb(0, 120, 255)), + egui::Stroke::new(1.5_f32, egui::Color32::from_rgb(0, 120, 255)), egui::StrokeKind::Outside, ); } @@ -719,7 +719,7 @@ fn draw_roi_selected_overlay( painter.rect_stroke( roi_rect, 0.0, - egui::Stroke::new(2.0, egui::Color32::from_rgb(0, 200, 80)), + egui::Stroke::new(2.0_f32, egui::Color32::from_rgb(0, 200, 80)), egui::StrokeKind::Outside, ); } @@ -742,7 +742,7 @@ fn draw_pixel_marker( rect.top() + frac_y * rect.height(), ); let r = 4.0; - let stroke = egui::Stroke::new(1.5, egui::Color32::from_rgb(255, 165, 0)); + let stroke = egui::Stroke::new(1.5_f32, egui::Color32::from_rgb(255, 165, 0)); painter.circle_stroke(center, r, stroke); painter.line_segment( [ diff --git a/apps/gui/src/widgets/statusbar.rs b/apps/gui/src/widgets/statusbar.rs index 548b61bb..73842b39 100644 --- a/apps/gui/src/widgets/statusbar.rs +++ b/apps/gui/src/widgets/statusbar.rs @@ -12,7 +12,7 @@ pub fn status_bar(ui: &mut egui::Ui, state: &AppState, rss_bytes: u64) { egui::Frame::NONE .fill(colors.bg2) .inner_margin(egui::Margin::symmetric(12, 4)) - .stroke(egui::Stroke::new(1.0, colors.border)), + .stroke(egui::Stroke::new(1.0_f32, colors.border)), ) .show_inside(ui, |ui| { ui.horizontal(|ui| { diff --git a/apps/gui/src/widgets/toolbar.rs b/apps/gui/src/widgets/toolbar.rs index 69313f1d..fb2df3f5 100644 --- a/apps/gui/src/widgets/toolbar.rs +++ b/apps/gui/src/widgets/toolbar.rs @@ -16,7 +16,7 @@ pub fn toolbar(ui: &mut egui::Ui, state: &mut AppState) { egui::Frame::NONE .fill(colors.bg2) .inner_margin(egui::Margin::symmetric(12, 6)) - .stroke(egui::Stroke::new(1.0, colors.border)), + .stroke(egui::Stroke::new(1.0_f32, colors.border)), ) .show_inside(ui, |ui| { ui.horizontal_centered(|ui| { diff --git a/crates/nereids-fitting/src/poisson.rs b/crates/nereids-fitting/src/poisson.rs index 355d9f70..6620a457 100644 --- a/crates/nereids-fitting/src/poisson.rs +++ b/crates/nereids-fitting/src/poisson.rs @@ -1260,6 +1260,17 @@ impl<'a> crate::forward_model::ForwardModel for CountsModel<'a> { /// Y(E) = α₁ · [Φ(E) · T(θ)] + α₂ · B(E) /// /// where `α₁` and `α₂` are parameter-vector entries. +/// +/// ## Index invariant +/// +/// `alpha1_index` / `alpha2_index` must NOT designate a parameter index +/// the transmission model reads. The wrapper cannot detect such a +/// collision through `dyn FitModel`, and the analytic Jacobian excludes +/// the scale indices from the inner free set — a collided parameter +/// would get only the scale contribution, silently omitting ∂T/∂p. +/// (Sharing ONE parameter between the two scale roles, +/// `alpha1_index == alpha2_index`, IS supported: the columns +/// accumulate.) pub struct CountsBackgroundScaleModel<'a> { /// Underlying transmission model. pub transmission_model: &'a dyn FitModel, @@ -1268,8 +1279,10 @@ pub struct CountsBackgroundScaleModel<'a> { /// Detector background spectrum. pub background: &'a [f64], /// Index of α₁ in the parameter vector. + /// Must not be a parameter the transmission model reads (see struct docs). pub alpha1_index: usize, /// Index of α₂ in the parameter vector. + /// Must not be a parameter the transmission model reads (see struct docs). pub alpha2_index: usize, /// Total parameter count in the wrapped model. pub n_params: usize, @@ -1342,14 +1355,20 @@ impl<'a> FitModel for CountsBackgroundScaleModel<'a> { return None; } + // Accumulate (+=) rather than assign: the struct does not forbid + // alpha1_index == alpha2_index, and evaluate() reads the aliased + // parameter for both roles, so its derivative is the SUM of both + // column contributions (f·t + b). With distinct indices each + // column is touched once and += on the zeroed matrix is identical + // to assignment. if let Some(col) = alpha1_col { for (row, (&f, &t)) in self.flux.iter().zip(t_inner.iter()).enumerate() { - *jacobian.get_mut(row, col) = f * t; + *jacobian.get_mut(row, col) += f * t; } } if let Some(col) = alpha2_col { for (row, &bg) in self.background.iter().enumerate() { - *jacobian.get_mut(row, col) = bg; + *jacobian.get_mut(row, col) += bg; } } @@ -1394,14 +1413,27 @@ impl<'a> crate::forward_model::ForwardModel for CountsBackgroundScaleModel<'a> { /// - ∂T_out/∂nₖ = ∂T_inner/∂nₖ = -σₖ(E)·T_inner(E) (same as bare model) /// - ∂T_out/∂b₀ = 1 /// - ∂T_out/∂b₁ = 1/√E +/// +/// ## Index invariant +/// +/// `b0_index` / `b1_index` must NOT designate a parameter index the +/// inner model reads. The wrapper cannot detect such a collision +/// through `dyn FitModel`, and the analytic Jacobian excludes the +/// background indices from the inner free set — a collided parameter +/// would get only the background contribution, silently omitting +/// ∂T_inner/∂p. (Sharing ONE parameter between the two background +/// roles, `b0_index == b1_index`, IS supported: the columns +/// accumulate.) pub struct TransmissionKLBackgroundModel<'a> { /// Underlying transmission model (density parameters only). pub inner: &'a dyn FitModel, /// Precomputed 1/√E for each energy bin. pub inv_sqrt_energies: Vec, /// Index of b₀ (constant background) in the parameter vector. + /// Must not be a parameter the inner model reads (see struct docs). pub b0_index: usize, /// Index of b₁ (1/√E background) in the parameter vector. + /// Must not be a parameter the inner model reads (see struct docs). pub b1_index: usize, /// Total parameter count in the wrapped model. pub n_params: usize, @@ -1454,7 +1486,7 @@ impl<'a> FitModel for TransmissionKLBackgroundModel<'a> { // Fill inner model columns (density, temperature). // Inner Jacobian is the same as bare model — background doesn't // affect ∂T_inner/∂nₖ. - if let Some(ref ij) = inner_jac { + if let Some(ij) = inner_jac.as_ref() { let mut inner_col = 0; for (col, &fp) in free_param_indices.iter().enumerate() { if fp == self.b0_index || fp == self.b1_index { @@ -1465,20 +1497,26 @@ impl<'a> FitModel for TransmissionKLBackgroundModel<'a> { } inner_col += 1; } - } else { - // No analytical inner Jacobian — fall back to FD for entire model. + } else if !inner_free.is_empty() { + // Inner params are free but the inner model has no analytical + // Jacobian — fall back to FD for the entire model. return None; } - // Background columns. + // Background columns. Accumulate (+=) rather than assign: the + // struct does not forbid b0_index == b1_index, and evaluate() + // reads the aliased parameter for both roles, so its derivative + // is the SUM of both column contributions (1 + 1/√E). With + // distinct indices each column is touched once and += on the + // zeroed matrix is identical to assignment. if let Some(col) = b0_col { for row in 0..n_e { - *jacobian.get_mut(row, col) = 1.0; // ∂T_out/∂b₀ = 1 + *jacobian.get_mut(row, col) += 1.0; // ∂T_out/∂b₀ = 1 } } if let Some(col) = b1_col { for row in 0..n_e { - *jacobian.get_mut(row, col) = self.inv_sqrt_energies[row]; // ∂T_out/∂b₁ = 1/√E + *jacobian.get_mut(row, col) += self.inv_sqrt_energies[row]; // ∂T_out/∂b₁ = 1/√E } } @@ -2404,4 +2442,266 @@ mod tests { true_params[3], ); } + + /// Inner params FREE + inner model WITHOUT `analytical_jacobian` → + /// the wrapper must return `None` (FD fallback for the entire model) + /// and the fit must still converge on the FD path. + #[test] + fn test_transmission_kl_background_fd_fallback_when_inner_lacks_jacobian() { + // ExponentialModel has no analytical_jacobian (trait default None). + // Counts-scale data (flux 1000, backgrounds 20/10) keeps the + // Poisson NLL well-conditioned for parameter recovery. + let x: Vec = (0..40).map(|i| 1.0 + 0.25 * i as f64).collect(); + let inner = ExponentialModel { + x: x.clone(), + flux: vec![1000.0; x.len()], + }; + let inv_sqrt_energies: Vec = x.iter().map(|&e| 1.0 / e.sqrt()).collect(); + let wrapped = TransmissionKLBackgroundModel { + inner: &inner, + inv_sqrt_energies, + b0_index: 1, + b1_index: 2, + n_params: 3, + }; + + let true_params = vec![0.4, 20.0, 10.0]; + let y_obs = wrapped.evaluate(&true_params).unwrap(); + + // Inner param 0 free (alone and together with b0/b1): no analytic + // inner Jacobian → wrapper falls back to FD. + assert!( + wrapped + .analytical_jacobian(&true_params, &[0, 1, 2], &y_obs) + .is_none(), + "inner param free without inner analytical_jacobian must give None" + ); + assert!( + wrapped + .analytical_jacobian(&true_params, &[0], &y_obs) + .is_none(), + "inner-only free set without inner analytical_jacobian must give None" + ); + + let mut params = ParameterSet::new(vec![ + FitParameter::non_negative("density", 0.8), + FitParameter { + name: "kl_b0".into(), + value: 5.0, + lower: 0.0, + upper: 500.0, + fixed: false, + }, + FitParameter { + name: "kl_b1".into(), + value: 5.0, + lower: 0.0, + upper: 500.0, + fixed: false, + }, + ]); + let config = PoissonConfig { + max_iter: 300, + ..PoissonConfig::default() + }; + let result = poisson_fit(&wrapped, &y_obs, &mut params, &config).unwrap(); + assert!( + result.converged, + "FD-fallback fit did not converge: {result:?}" + ); + assert!( + (result.params[0] - true_params[0]).abs() / true_params[0] < 0.05, + "density fit={}, true={}", + result.params[0], + true_params[0], + ); + } + + /// Inner params FIXED (only b0/b1 free) → the wrapper stays on the + /// analytic path: `analytical_jacobian` returns `Some` with the exact + /// background columns ∂T/∂b₀ = 1 and ∂T/∂b₁ = 1/√E, and the + /// background-only fit converges. + #[test] + fn test_transmission_kl_background_background_only_analytic_jacobian() { + // Counts-scale data — see the FD-fallback test above. + let x: Vec = (0..40).map(|i| 1.0 + 0.25 * i as f64).collect(); + let inner = ExponentialModel { + x: x.clone(), + flux: vec![1000.0; x.len()], + }; + let inv_sqrt_energies: Vec = x.iter().map(|&e| 1.0 / e.sqrt()).collect(); + let wrapped = TransmissionKLBackgroundModel { + inner: &inner, + inv_sqrt_energies: inv_sqrt_energies.clone(), + b0_index: 1, + b1_index: 2, + n_params: 3, + }; + + let true_params = vec![0.4, 20.0, 10.0]; + let y_obs = wrapped.evaluate(&true_params).unwrap(); + + let jac = wrapped + .analytical_jacobian(&true_params, &[1, 2], &y_obs) + .expect("background-only free set must stay on the analytic path"); + for (row, &inv_sqrt_e) in inv_sqrt_energies.iter().enumerate() { + assert!( + (jac.get(row, 0) - 1.0).abs() < 1e-15, + "∂T/∂b₀ at row {row} = {}, expected 1.0", + jac.get(row, 0), + ); + assert!( + (jac.get(row, 1) - inv_sqrt_e).abs() < 1e-15, + "∂T/∂b₁ at row {row} = {}, expected {inv_sqrt_e}", + jac.get(row, 1), + ); + } + + // Background-only fit (density fixed at truth) converges on the + // analytic path. + let mut params = ParameterSet::new(vec![ + FitParameter::fixed("density", true_params[0]), + FitParameter { + name: "kl_b0".into(), + value: 5.0, + lower: 0.0, + upper: 500.0, + fixed: false, + }, + FitParameter { + name: "kl_b1".into(), + value: 5.0, + lower: 0.0, + upper: 500.0, + fixed: false, + }, + ]); + let config = PoissonConfig { + max_iter: 300, + ..PoissonConfig::default() + }; + let result = poisson_fit(&wrapped, &y_obs, &mut params, &config).unwrap(); + assert!( + result.converged, + "background-only analytic fit did not converge: {result:?}" + ); + assert!( + (result.params[1] - true_params[1]).abs() < 0.1, + "b0 fit={}, true={}", + result.params[1], + true_params[1], + ); + assert!( + (result.params[2] - true_params[2]).abs() < 0.1, + "b1 fit={}, true={}", + result.params[2], + true_params[2], + ); + } + + /// Central finite-difference column for one parameter, computed + /// straight from `evaluate` — an oracle independent of the model's + /// analytic Jacobian path. + fn fd_column(model: &dyn FitModel, params: &[f64], param_index: usize, h: f64) -> Vec { + let mut plus = params.to_vec(); + plus[param_index] += h; + let mut minus = params.to_vec(); + minus[param_index] -= h; + let y_plus = model.evaluate(&plus).unwrap(); + let y_minus = model.evaluate(&minus).unwrap(); + y_plus + .iter() + .zip(y_minus.iter()) + .map(|(&p, &m)| (p - m) / (2.0 * h)) + .collect() + } + + /// Aliased background indices (b0_index == b1_index): the analytic + /// Jacobian must ACCUMULATE both roles' contributions (1 + 1/√E), + /// matching finite differences — not overwrite one with the other. + #[test] + fn test_transmission_kl_background_aliased_indices_jacobian_matches_fd() { + let x: Vec = (0..10).map(|i| 1.0 + 0.5 * i as f64).collect(); + let inner = ExponentialModel { + x: x.clone(), + flux: vec![1000.0; x.len()], + }; + let inv_sqrt_energies: Vec = x.iter().map(|&e| 1.0 / e.sqrt()).collect(); + let wrapped = TransmissionKLBackgroundModel { + inner: &inner, + inv_sqrt_energies: inv_sqrt_energies.clone(), + b0_index: 1, + b1_index: 1, // deliberately aliased with b0 + n_params: 2, + }; + + let params = vec![0.4, 15.0]; + let y = wrapped.evaluate(¶ms).unwrap(); + // Inner param fixed, only the aliased background param free → + // analytic path. + let jac = wrapped + .analytical_jacobian(¶ms, &[1], &y) + .expect("background-only free set must stay on the analytic path"); + + let fd = fd_column(&wrapped, ¶ms, 1, 1e-6); + for (row, (&fd_val, &inv_sqrt_e)) in fd.iter().zip(inv_sqrt_energies.iter()).enumerate() { + let expected = 1.0 + inv_sqrt_e; + assert!( + (jac.get(row, 0) - expected).abs() < 1e-12, + "aliased ∂/∂b at row {row}: analytic {}, expected {expected}", + jac.get(row, 0), + ); + assert!( + (jac.get(row, 0) - fd_val).abs() < 1e-5, + "aliased ∂/∂b at row {row}: analytic {}, FD {fd_val}", + jac.get(row, 0), + ); + } + } + + /// Aliased scale indices (alpha1_index == alpha2_index) in the + /// sibling counts model: same accumulate-not-overwrite requirement, + /// derivative f·T + B against the finite-difference oracle. + #[test] + fn test_counts_background_scale_aliased_indices_jacobian_matches_fd() { + let x: Vec = (0..10).map(|i| 1.0 + 0.5 * i as f64).collect(); + let inner = ExponentialModel { + x: x.clone(), + flux: vec![1.0; x.len()], // inner transmission in [0,1] + }; + let flux: Vec = vec![1000.0; x.len()]; + let background: Vec = x.iter().map(|&e| 5.0 + e).collect(); + let wrapped = CountsBackgroundScaleModel { + transmission_model: &inner, + flux: &flux, + background: &background, + alpha1_index: 1, + alpha2_index: 1, // deliberately aliased with alpha1 + n_params: 2, + }; + + let params = vec![0.4, 1.2]; + let y = wrapped.evaluate(¶ms).unwrap(); + let t_inner = inner.evaluate(¶ms).unwrap(); + // Inner param fixed, only the aliased scale param free → + // analytic path. + let jac = wrapped + .analytical_jacobian(¶ms, &[1], &y) + .expect("scale-only free set must stay on the analytic path"); + + let fd = fd_column(&wrapped, ¶ms, 1, 1e-6); + for (row, &fd_val) in fd.iter().enumerate() { + let expected = flux[row] * t_inner[row] + background[row]; + assert!( + (jac.get(row, 0) - expected).abs() < 1e-9, + "aliased ∂/∂α at row {row}: analytic {}, expected {expected}", + jac.get(row, 0), + ); + assert!( + (jac.get(row, 0) - fd_val).abs() < 1e-3, + "aliased ∂/∂α at row {row}: analytic {}, FD {fd_val}", + jac.get(row, 0), + ); + } + } } diff --git a/crates/nereids-fitting/src/transmission_model.rs b/crates/nereids-fitting/src/transmission_model.rs index 0b7011fb..e9dabee7 100644 --- a/crates/nereids-fitting/src/transmission_model.rs +++ b/crates/nereids-fitting/src/transmission_model.rs @@ -1244,6 +1244,13 @@ impl FitModel for TransmissionFitModel { let t_for_deriv: &[f64] = t_unresolved.as_deref().unwrap_or(y_current); // ── Density columns: ∂T/∂N_g or ∂T_obs/∂N_g ── + // Role indices are assumed DISTINCT (first-match layout: the + // temperature column is skipped here and filled separately, so a + // parameter serving both roles would get only one contribution). + // The pipeline always constructs distinct indices; aliasing is + // not supported in this resolution-coupled fill — see + // NormalizedTransmissionModel's "Index invariant" for the + // accumulate-hardened pattern used by the simple wrappers. for (col, &fp_idx) in free_param_indices.iter().enumerate() { if temp_col == Some(col) { continue; @@ -1361,6 +1368,15 @@ impl FitModel for TransmissionFitModel { /// SAMMY fits up to 6 background terms; we implement all 6: /// Anorm, constant BackA, 1/√E term BackB, √E term BackC, /// exponential amplitude BackD, exponential decay BackF. +/// +/// ## Index invariant +/// +/// The role indices (`anorm_index`, `back_*_index`) must NOT designate +/// a parameter the inner model reads: the analytic Jacobian filters the +/// role indices out of the inner free set, so such a collision cannot +/// be detected and the column would silently omit Anorm × ∂T_inner/∂p. +/// Aliasing AMONG the role indices themselves IS supported — the +/// Jacobian columns accumulate. pub struct NormalizedTransmissionModel { /// The inner (pure Beer-Lambert) transmission model. inner: M, @@ -1567,33 +1583,53 @@ impl FitModel for NormalizedTransmissionModel { inner_col_map.insert(idx, col); } + // Independent role checks with accumulation (+=) rather than a + // first-match if/else-if chain: nothing forbids two role indices + // from aliasing, and evaluate() reads an aliased parameter for + // every role it occupies, so its derivative is the SUM of the + // matching columns. Distinct indices touch each column once on a + // zeroed matrix — identical to assignment. A role index colliding + // with an INNER-model parameter remains undetectable here (role + // indices are filtered out of inner_free_indices) — see the + // struct docs. for (col, &fp_idx) in free_param_indices.iter().enumerate() { + let mut matched = false; if fp_idx == self.anorm_index { // ∂T_out/∂Anorm = T_inner(E) for (i, &ti) in t_inner.iter().enumerate() { - *jacobian.get_mut(i, col) = ti; + *jacobian.get_mut(i, col) += ti; } - } else if fp_idx == self.back_a_index { + matched = true; + } + if fp_idx == self.back_a_index { // ∂T_out/∂BackA = 1 for i in 0..n_e { - *jacobian.get_mut(i, col) = 1.0; + *jacobian.get_mut(i, col) += 1.0; } - } else if fp_idx == self.back_b_index { + matched = true; + } + if fp_idx == self.back_b_index { // ∂T_out/∂BackB = 1/√E for (i, &inv_se) in self.inv_sqrt_energies.iter().enumerate() { - *jacobian.get_mut(i, col) = inv_se; + *jacobian.get_mut(i, col) += inv_se; } - } else if fp_idx == self.back_c_index { + matched = true; + } + if fp_idx == self.back_c_index { // ∂T_out/∂BackC = √E for (i, &se) in self.sqrt_energies.iter().enumerate() { - *jacobian.get_mut(i, col) = se; + *jacobian.get_mut(i, col) += se; } - } else if self.back_d_index == Some(fp_idx) { + matched = true; + } + if self.back_d_index == Some(fp_idx) { // ∂T_out/∂BackD = exp(−BackF / √E) for (i, &et) in exp_terms.iter().enumerate() { - *jacobian.get_mut(i, col) = et; + *jacobian.get_mut(i, col) += et; } - } else if self.back_f_index == Some(fp_idx) { + matched = true; + } + if self.back_f_index == Some(fp_idx) { // ∂T_out/∂BackF = −BackD × exp(−BackF / √E) / √E let back_d = params[self.back_d_index.unwrap()]; for (i, (&et, &inv_se)) in exp_terms @@ -1601,20 +1637,24 @@ impl FitModel for NormalizedTransmissionModel { .zip(self.inv_sqrt_energies.iter()) .enumerate() { - *jacobian.get_mut(i, col) = -back_d * et * inv_se; + *jacobian.get_mut(i, col) += -back_d * et * inv_se; } - } else if let Some(&inner_col) = inner_col_map.get(&fp_idx) { + matched = true; + } + if let Some(&inner_col) = inner_col_map.get(&fp_idx) { // Inner model parameter: ∂T_out/∂p = Anorm × ∂T_inner/∂p if let Some(ref jac) = inner_jac { for i in 0..n_e { - *jacobian.get_mut(i, col) = anorm * jac.get(i, inner_col); + *jacobian.get_mut(i, col) += anorm * jac.get(i, inner_col); } + matched = true; } else { // Inner model did not provide analytical Jacobian — // fall back to finite-difference for the whole thing. return None; } - } else { + } + if !matched { // Unknown parameter — should not happen, but fall back to FD. return None; } @@ -2392,6 +2432,11 @@ impl FitModel for EnergyScaleTransmissionModel { // byte-identically to `apply_resolution`. let density_plan = self.cached_resolution_plan(t0, l_scale, work_e); + // Role indices (t0/L_scale/temperature/densities) are assumed + // DISTINCT — first-match layout; aliasing is not supported in + // this FD-arm fill. See NormalizedTransmissionModel's "Index + // invariant" for the accumulate-hardened pattern used by the + // simple wrappers. for (col, &fp_idx) in free_param_indices.iter().enumerate() { // Temperature (issue #634), when free, is differentiated by the // per-coordinate central FD arm below — it is neither t0 nor @@ -2790,6 +2835,15 @@ pub fn baseline_reference_energy_active(energies: &[f64], active: Option<&[bool] /// supporting it buys nothing (`Anorm` would just play `b0`'s role at a /// rescaled value) and splits the normalization story across two knobs; /// the sanctioned combination is `Anorm` held fixed. +/// +/// ## Index invariant +/// +/// The baseline indices (`b0_index`, `b1_index`, `b2_index`) must NOT +/// designate a parameter the inner model reads: the analytic Jacobian +/// filters the baseline indices out of the inner free set, so such a +/// collision cannot be detected and the column would silently omit +/// B(E) × ∂T_inner/∂p. Aliasing AMONG the baseline indices themselves +/// IS supported — the Jacobian columns accumulate. pub struct MultiplicativeBaselineModel { /// The inner model (bare transmission, or the additive-background /// wrapper when both are configured). @@ -2936,34 +2990,52 @@ impl FitModel for MultiplicativeBaselineModel { } let mut jacobian = FlatMatrix::zeros(n_e, n_free); + // Independent role checks with accumulation (+=) rather than a + // first-match if/else-if chain: nothing forbids the baseline + // indices from aliasing, and baseline_at() reads an aliased + // parameter for every role it occupies, so its derivative is the + // SUM of the matching columns. Distinct indices touch each column + // once on a zeroed matrix — identical to assignment. A baseline + // index colliding with an INNER-model parameter remains + // undetectable here (baseline indices are filtered out of + // inner_free_indices) — see the struct docs. for (col, &fp_idx) in free_param_indices.iter().enumerate() { + let mut matched = false; if fp_idx == self.b0_index { // ∂y/∂b0 = T_inner for (i, &ti) in t_inner.iter().enumerate() { - *jacobian.get_mut(i, col) = ti; + *jacobian.get_mut(i, col) += ti; } - } else if fp_idx == self.b1_index { + matched = true; + } + if fp_idx == self.b1_index { // ∂y/∂b1 = z · T_inner for (i, &ti) in t_inner.iter().enumerate() { - *jacobian.get_mut(i, col) = self.ln_ratio[i] * ti; + *jacobian.get_mut(i, col) += self.ln_ratio[i] * ti; } - } else if fp_idx == self.b2_index { + matched = true; + } + if fp_idx == self.b2_index { // ∂y/∂b2 = z² · T_inner for (i, &ti) in t_inner.iter().enumerate() { - *jacobian.get_mut(i, col) = self.ln_ratio_sq[i] * ti; + *jacobian.get_mut(i, col) += self.ln_ratio_sq[i] * ti; } - } else if let Some(&inner_col) = inner_col_map.get(&fp_idx) { + matched = true; + } + if let Some(&inner_col) = inner_col_map.get(&fp_idx) { // Inner parameter: ∂y/∂p = B(E) · ∂T_inner/∂p if let Some(ref jac) = inner_jac { for i in 0..n_e { let b = self.baseline_at(params, i); - *jacobian.get_mut(i, col) = b * jac.get(i, inner_col); + *jacobian.get_mut(i, col) += b * jac.get(i, inner_col); } + matched = true; } else { // Inner has no analytic Jacobian — FD for everything. return None; } - } else { + } + if !matched { // Unknown parameter — should not happen; fall back to FD. return None; } @@ -4580,6 +4652,103 @@ mod tests { } } + /// Aliased role indices (BackA == BackB sharing one parameter): the + /// analytic Jacobian must ACCUMULATE both roles' contributions + /// (1 + 1/√E), matching central finite differences — not keep only + /// the first match. + #[test] + fn normalized_jacobian_aliased_role_indices_match_fd() { + let xs = vec![vec![1.0, 2.0, 3.0]]; + let inner = make_precomputed(xs, vec![0]); + + let energies = [4.0, 9.0, 16.0]; + // params: [density, Anorm, BackAB (shared), BackC] — BackA and + // BackB deliberately alias index 2. + let model = NormalizedTransmissionModel::new(inner, &energies, 1, 2, 2, 3); + + let params = [0.3, 0.95, 0.02, 0.005]; + let y = model.evaluate(¶ms).unwrap(); + let free: Vec = (0..4).collect(); + + let jac = model + .analytical_jacobian(¶ms, &free, &y) + .expect("analytical_jacobian should return Some"); + + let h = 1e-7; + for (col, &p_idx) in free.iter().enumerate() { + let mut p_plus = params; + let mut p_minus = params; + p_plus[p_idx] += h; + p_minus[p_idx] -= h; + let y_plus = model.evaluate(&p_plus).unwrap(); + let y_minus = model.evaluate(&p_minus).unwrap(); + for i in 0..3 { + let fd = (y_plus[i] - y_minus[i]) / (2.0 * h); + let ana = jac.get(i, col); + let err = (fd - ana).abs(); + let scale = fd.abs().max(ana.abs()).max(1e-10); + assert!( + err / scale < 1e-4, + "aliased Jacobian mismatch (row {i}, col {col}): FD={fd:.8}, \ + analytical={ana:.8}", + ); + } + } + // Pin the aliased column exactly: ∂/∂BackAB = 1 + 1/√E. + for (i, &e) in energies.iter().enumerate() { + let expected = 1.0 + 1.0 / e.sqrt(); + assert!( + (jac.get(i, 2) - expected).abs() < 1e-12, + "aliased column row {i}: {} vs expected {expected}", + jac.get(i, 2), + ); + } + } + + /// Aliased baseline indices (b0 == b1 sharing one parameter) in the + /// multiplicative wrapper: same accumulate-not-overwrite requirement, + /// derivative (1 + z)·T_inner against the FD oracle. + #[test] + fn multiplicative_baseline_aliased_indices_match_fd() { + let xs = vec![vec![1.0, 2.0, 3.0]]; + let inner = make_precomputed(xs, vec![0]); + + let energies = [4.0, 9.0, 16.0]; + let e_ref = baseline_reference_energy(&energies); + // params: [density, b01 (shared), b2] — b0 and b1 deliberately + // alias index 1. + let model = MultiplicativeBaselineModel::new(inner, &energies, e_ref, 1, 1, 2); + + let params = [0.3, 1.02, 0.01]; + let y = model.evaluate(¶ms).unwrap(); + let free: Vec = (0..3).collect(); + + let jac = model + .analytical_jacobian(¶ms, &free, &y) + .expect("analytical_jacobian should return Some"); + + let h = 1e-7; + for (col, &p_idx) in free.iter().enumerate() { + let mut p_plus = params; + let mut p_minus = params; + p_plus[p_idx] += h; + p_minus[p_idx] -= h; + let y_plus = model.evaluate(&p_plus).unwrap(); + let y_minus = model.evaluate(&p_minus).unwrap(); + for i in 0..3 { + let fd = (y_plus[i] - y_minus[i]) / (2.0 * h); + let ana = jac.get(i, col); + let err = (fd - ana).abs(); + let scale = fd.abs().max(ana.abs()).max(1e-10); + assert!( + err / scale < 1e-4, + "aliased baseline Jacobian mismatch (row {i}, col {col}): FD={fd:.8}, \ + analytical={ana:.8}", + ); + } + } + } + /// Verify that when some background params are fixed (not in /// free_param_indices), the Jacobian columns are correct. #[test] diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 00000000..9c5b9da5 --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,31 @@ +# Pinned Rust toolchain — single source of truth for CI and rustup users. +# +# Why pinned: CI used to float `stable`, so every new stable release +# (with its new clippy/rustc lints under `-D warnings`) instantly turned +# ALL open PRs red for reasons unrelated to their diffs (2026-07-10: +# Rust 1.97.0 reddened 11 Dependabot PRs at once). Pinning makes +# toolchain bumps deliberate, reviewed changes. +# +# Why 1.96.1 and not 1.97.0: rust-lang/rust#159035 — a P-critical +# x86-64 miscompile (optimized safe code segfaults). The underlying +# LLVM codegen issue predates 1.97 (LLVM 20 era, ~1.87+), but 1.97.0's +# enum-tag change (rust-lang/rust#155850) makes the trigger condition +# far more common; the issue's reproducer passes on 1.96.1. Release +# wheels for the ORNL x86-64 RHEL fleet are built under this pin: +# 1.96.1 matches what main already shipped with and avoids the widened +# 1.97 trigger. Bump to the first stable containing the #155850 revert +# (rust-lang/rust#159047) and/or the upstream LLVM fix. +# +# Bump ritual: update `channel` here, update the matching +# `toolchain: "X.Y.Z"` inputs in .github/workflows/*.yml (they only +# pre-install the right version — for ordinary invocations in this +# repo, without a higher-precedence rustup override such as +# `cargo +toolchain`, `RUSTUP_TOOLCHAIN`, or a nearer directory +# override, the rustup proxies enforce THIS file), fix any new lints, +# one PR. +# +# Note: maturin-action's manylinux containers (publish.yml, +# wheel-policy.yml) also honor this file and will download the pinned +# toolchain inside the container — intended, for reproducible wheels. +[toolchain] +channel = "1.96.1"