Skip to content

refactor(mixed): RAII guard for proposed-state OMRF marginal evaluation#137

Merged
MaartenMarsman merged 1 commit into
mainfrom
refactor/mixed-proposed-state-raii
Jun 4, 2026
Merged

refactor(mixed): RAII guard for proposed-state OMRF marginal evaluation#137
MaartenMarsman merged 1 commit into
mainfrom
refactor/mixed-proposed-state-raii

Conversation

@MaartenMarsman

Copy link
Copy Markdown
Collaborator

What

The three continuous Kyy moves — update_pairwise_effects_continuous_offdiag, update_pairwise_effects_continuous_diag, and update_edge_indicator_continuous — each evaluate the OMRF marginal log-likelihood under the proposed (Kyy', Σ') by temporarily setting pairwise_effects_continuous_, covariance_continuous_, and marginal_interactions_, summing log_marginal_omrf, then restoring the accepted state. This was open-coded as three near-identical save / mutate / recompute / restore blocks (offdiag and edge-indicator byte-identical; diag saved a single element).

This replaces the manual save/restore with a file-local ProposedContinuousState RAII guard that snapshots the three fields on construction and restores them on scope exit. Each call site becomes a { } block: construct guard → set proposed values → recompute_marginal_interactions() → sum marginals → (scope exit restores).

Removes the manual-restore footgun — any future field touched inside the block can no longer leak past it by a forgotten restore.

Why this is safe

Same fields mutated, same recompute_marginal_interactions(), same values restored. Restore order among the three independent fields is irrelevant. The diag case now snapshots the whole pairwise_effects_continuous_ matrix rather than the single (i,i) element, but only (i,i) is ever changed in the block, so the restored matrix is identical.

Verified bitwise: adaptive-Metropolis mixed-MRF raw draws (main + pairwise + indicator) are digest-identical pre/post across edge-sel delta=0, no-edge delta=0, and no-edge delta=1. identical(main, branch) == TRUE. Harness: dev/bitwise_mixed_logdet_ratio.R.

The three continuous Kyy moves (update_pairwise_effects_continuous_offdiag,
_diag, and update_edge_indicator_continuous) each temporarily set
pairwise_effects_continuous_, covariance_continuous_, and marginal_interactions_
to proposed values, sum the OMRF marginals, then restore the accepted state --
open-coded as three near-identical save / mutate / recompute / restore blocks
(offdiag and edge-indicator byte-identical; diag saved one element).

Replace the manual save/restore with a file-local ProposedContinuousState RAII
guard that snapshots the three fields on construction and restores them on
scope exit, so the proposed-state evaluation lives in a { } block that cannot
leak the mutation regardless of control flow.

Behavior-preserving: same fields mutated, same recompute, same values restored
(restore order among the independent fields is irrelevant; the diag case now
snapshots the whole pairwise matrix but only (i,i) is changed, so the restored
state is identical).

Verified bitwise: adaptive-Metropolis mixed-MRF raw draws (edge-sel delta=0,
no-edge delta=0, no-edge delta=1) digest-identical pre/post.
@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.33333% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.72%. Comparing base (44efd57) to head (ec419cb).

Files with missing lines Patch % Lines
src/models/mixed/mixed_mrf_metropolis.cpp 53.33% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
+ Coverage   87.61%   87.72%   +0.10%     
==========================================
  Files          87       87              
  Lines       12891    12883       -8     
==========================================
+ Hits        11295    11301       +6     
+ Misses       1596     1582      -14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MaartenMarsman MaartenMarsman merged commit e931dc4 into main Jun 4, 2026
9 of 10 checks passed
@MaartenMarsman MaartenMarsman deleted the refactor/mixed-proposed-state-raii branch June 4, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant