Skip to content

refactor(ggm): share active<->full index traversal via for_each_active_full_pair#139

Merged
MaartenMarsman merged 1 commit into
mainfrom
refactor/ggm-active-full-scatter-gather
Jun 4, 2026
Merged

refactor(ggm): share active<->full index traversal via for_each_active_full_pair#139
MaartenMarsman merged 1 commit into
mainfrom
refactor/ggm-active-full-scatter-gather

Conversation

@MaartenMarsman

Copy link
Copy Markdown
Collaborator

What

get_full_vectorized_parameters (scatters active theta into the full zero-padded vector) and get_active_inv_mass (gathers the full-dim inverse mass back into the active layout) open-coded the same per-column index arithmetic in inverse directions — the same theta_offsets/full_theta_offsets/included_indices/psi_offset/full_psi_offset walk, written twice.

Adds GraphConstraintStructure::for_each_active_full_pair(fn), which visits each active entry once as (active_index, full_index) — the d_q free off-diagonal entries then psi_q, per column. The scatter and gather are now one-line lambdas over it:

cs.for_each_active_full_pair([&](size_t a, size_t f){ full(f)   = theta_(a);    }); // scatter
cs.for_each_active_full_pair([&](size_t a, size_t f){ active(a) = inv_mass_(f); }); // gather

The two are now provably exact inverses (same index pairs), which the old duplicated loops only implied.

Why this is safe

Identical (active, full) index pairs in identical order; the assignments are unchanged. psi_offset(q) == theta_offsets[q] + d_q, so the helper's psi pair matches the old scatter's theta_(active_offset + col.d_q).

Verified bitwise: update_method="nuts" GGM fits with diagonal mass adaptation (which is what exercises these two functions — scatter feeds Welford during warmup, gather returns the adapted mass), edge-selection (sparse active dim, exercises included_indices) and no-edge. identical(main, branch) == TRUE. Harness: dev/bitwise_ggm_nuts.R.

…e_full_pair

get_full_vectorized_parameters (scatter active theta -> full zero-padded vector)
and get_active_inv_mass (gather full-dim inverse mass -> active layout) open-coded
the same per-column index arithmetic in inverse directions. Add
GraphConstraintStructure::for_each_active_full_pair(fn), which visits each active
entry as (active_index, full_index); the scatter and gather become one-line
lambdas over it.

Behavior-preserving: identical (active,full) index pairs in identical order; the
assignments are unchanged.

Verified bitwise: NUTS GGM fits with mass adaptation (edge-selection exercising
included_indices, and no-edge) digest-identical pre/post.
@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.73%. Comparing base (07b1137) to head (e9b99ae).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   87.71%   87.73%   +0.01%     
==========================================
  Files          87       87              
  Lines       12867    12861       -6     
==========================================
- Hits        11286    11283       -3     
+ Misses       1581     1578       -3     

☔ 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 9e21f06 into main Jun 4, 2026
10 checks passed
@MaartenMarsman MaartenMarsman deleted the refactor/ggm-active-full-scatter-gather branch June 4, 2026 18:24
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