Implement Balanced Up-Down Walk (Akitaya et al 2026) by christopherkenny · Pull Request #211 · alarm-redist/redist

christopherkenny · 2026-03-02T19:26:36Z

This builds off of the Cycle Walk implementation, so the code is a bit less spread across files.

Paper is from a few weeks ago: https://arxiv.org/abs/2602.11993

Covered here are:

standard algorithm
weighted edge support using the same approach as cyclewalk
integration with our MH system
consistency with our diagnostics
support for shortbursts
consistent tests + validations with cyclewalk

7x7 validation:

The root cause was that get_district_path returned DistrictPair{min,max}, which loses the directed order of the district path. For a path like 2->1->0, edges were stored as {1,2},{0,1} causing districts to be extracted as [1,0,1] instead of [2,1,0]. This led to duplicate district assignments, silently dropped marked edges, and systematic bias. Fix: Store district_path as directed pair<int,int> preserving {from,to} order. Convert to DistrictPair only for marked_edges map lookup. Other changes: - Add BUD diagnostics (accept/reject/trivial) to summary output - Add 'bud' to supported algorithms in redist_ci - Remove debug-only compactness==98/99 special modes - Remove compute_log_wst_from_plan dead code - Add translations.md documenting Julia-to-C++ review findings

Copilot

Pull request overview

This PR adds a new MCMC backend implementing the “Balanced Up-Down Walk (BUD)” sampler and integrates it into the existing redist workflow (shortburst optimization, diagnostics, edge-weight validation, and tests), largely mirroring the existing CycleWalk structure.

Changes:

Introduces a full BUD sampler stack (C++ core + R wrapper) with support for constraints, compactness, multiple chains, and edge-weight validation.
Integrates BUD into redist_shortburst(), summary.redist_plans(), and redist_ci().
Adds a comprehensive test suite (including long-run distribution checks, skipped on CRAN/CI) and exploratory validation script(s).

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/testthat/test_burst.R	Adds shortburst tests for the new `backend = "bud"`.
tests/testthat/test-redist_bud_weights.R	Adds edge-weight validation + behavior tests for `redist_bud()`.
tests/testthat/test-redist_bud_long.R	Adds long-running distribution validation tests for BUD (skipped on CRAN/CI).
tests/testthat/test-redist_bud_chains.R	Adds multi-chain and init-plan behavior tests for BUD.
tests/testthat/test-redist_bud.R	Adds core correctness/sanity tests for BUD sampling output.
src/cw_partition.h	Makes `LCTPartition` internals `protected` for subclassing (needed by BUD partition).
src/cw_lct.cpp	Reworks reversal propagation to be iterative and adds loop guards.
src/bud_proposal.h	Declares BUD proposal/step interfaces and data structures.
src/bud_proposal.cpp	Implements the BUD proposal and MH step, including compactness/constraint adjustments.
src/bud_partition.h	Adds BUD-specific partition state (district tree, marked edges, cycle LCT, cuttable info).
src/bud_partition.cpp	Implements BUD partition initialization and cuttable-info computations.
src/bud_main.h	Declares the BUD entry point for the Rcpp-exposed MCMC loop.
src/bud_main.cpp	Implements the BUD MCMC loop (`bud_plans`) analogous to CycleWalk.
src/RcppExports.cpp	Registers `_redist_bud_plans` for R access.
man/redist_shortburst.Rd	Documents new `"bud"` backend option in shortburst.
man/redist_bud.Rd	Adds man page for `redist_bud()`.
explore/validate_bud_7x7.R	Adds an exploratory validation script for 7x7 diagnostics/plots.
R/redist_shortburst.R	Adds `"bud"` backend and calls into `bud_plans()`.
R/redist_bud.R	Adds the public R wrapper for BUD with chains/warmup/thin/edge-weights/constraints.
R/diagnostics.R	Extends summary output to include BUD-specific diagnostics.
R/confint.R	Allows `redist_ci()` to accept `"bud"` algorithm outputs.
R/RcppExports.R	Adds the R binding for `bud_plans()`.
NAMESPACE	Exports `redist_bud`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-07T21:53:52Z

+    int guard = 0;
    while (n->path_parent != nullptr) {
+        if (++guard > 10000) {
+            throw std::runtime_error("LCT expose: infinite loop detected");
+        }


This loop guard is hard-coded to 10000; for large instances it may throw even when path_parent chains are valid. Prefer a bound derived from the number of nodes (size()) or explicit cycle detection.

Copilot · 2026-03-07T21:53:52Z

+    int guard = 0;
    while (r->children[0] != nullptr) {
+        if (++guard > 10000) {
+            throw std::runtime_error("LCT find_root: infinite loop detected");
+        }


The 10000-iteration guard can fail on large graphs where the left spine of the splay tree legitimately exceeds 10k. Use a bound tied to size() or explicit cycle detection instead of a constant.

Copilot · 2026-03-07T21:53:53Z

+    int guard = 0;
    while (root->parent != nullptr) {
+        if (++guard > 10000) {
+            throw std::runtime_error("LCT find_path: infinite loop detected");
+        }


The hard-coded guard (10000) can throw on valid large trees. If the intent is to prevent infinite loops, prefer guarding by size() or detecting cycles explicitly so this remains correct at scale.

Copilot · 2026-03-07T21:53:53Z

+    if (randSamp > bcuts.pathWeights[bcuts.pathWeights.size() - 2]) {
+        cut_district_path(partition, district_path);
+        return {0.0, update};
+    }


This check rejects proposals whenever randSamp falls in the last interval of pathWeights (and if there is only 1 balanced cut, it rejects 100% of the time). Since cumWeight includes all balanced cuts, this introduces an unintended rejection probability and can prevent any proposal from being selected. Likely fix: remove this guard entirely, or compare against the final cumulative weight (pathWeights.back()) / adjust the sampling logic so every balanced cut has a chance to be selected.

Suggested change

if (randSamp > bcuts.pathWeights[bcuts.pathWeights.size() - 2]) {

cut_district_path(partition, district_path);

return {0.0, update};

}

Copilot · 2026-03-07T21:53:54Z

+    while (cur != nullptr) {
+        if (++guard > 10000) {


The hard-coded guard limit (10000) can throw on legitimately large splay trees / large maps (e.g., >10k vertices) even when the structure is valid. If you want a safety check against cycles, prefer guarding by the maximum possible chain length (e.g., size()), or detect cycles explicitly (visited set / tortoise-hare), rather than a fixed constant.

Suggested change

while (cur != nullptr) {

if (++guard > 10000) {

int maxSteps = size();

while (cur != nullptr) {

if (++guard > maxSteps) {

Copilot · 2026-03-07T21:53:54Z

+    int guard = 0;
    while (n->parent != nullptr) {
+        if (++guard > 10000) {
+            throw std::runtime_error("LCT splay: infinite loop detected");
+        }


The fixed iteration guard (10000) can incorrectly trigger on valid inputs for large trees/maps. Consider bounding by size() (or a stricter proven invariant), or switch to explicit cycle detection so this never throws purely due to scale.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

christopherkenny added 10 commits February 28, 2026 00:09

start w tests [ci skip]

39c3c5b

old red changes

1dce78d

shorten tests

6397cce

cw updpates to allow bud

0e206fb

bud, not yet validating

6d21ca7

add bud to shortburst

2d2ca1b

typo

4417fed

match cw for weights

081b58a

Merge branch 'dev' into bud

bd08598

christopherkenny marked this pull request as ready for review March 7, 2026 21:44

christopherkenny requested review from CoryMcCartan and Copilot March 7, 2026 21:45

Copilot started reviewing on behalf of christopherkenny March 7, 2026 21:45 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

christopherkenny and others added 2 commits March 23, 2026 08:13

Update R/redist_shortburst.R

2e505f8

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update man/redist_shortburst.Rd

55ffe33

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement Balanced Up-Down Walk (Akitaya et al 2026)#211

Implement Balanced Up-Down Walk (Akitaya et al 2026)#211
christopherkenny wants to merge 12 commits into
devfrom
bud

christopherkenny commented Mar 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

christopherkenny commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

christopherkenny commented Mar 2, 2026 •

edited

Loading