Skip to content

Sbm bug fix - #219

Open
sekulovskin wants to merge 2 commits into
Bayesian-Graphical-Modelling-Lab:mainfrom
sekulovskin:sbm_bug_fix
Open

sekulovskin wants to merge 2 commits into
Bayesian-Graphical-Modelling-Lab:mainfrom
sekulovskin:sbm_bug_fix

Conversation

@sekulovskin

Copy link
Copy Markdown
Contributor

Description

Fixes how the SBM edge prior sets the inclusion probabilities of a newly opened
cluster. They were drawn from the prior instead of from their posterior given the
variable's edges. As a result, the sampler did not target the correct posterior
over partitions.

Problem / Motivation

In the allocation step (block_allocations_mfm_sbm), a variable that leaves a
shared cluster to start a new one is scored with the correct integrated
likelihood. But the new cluster's between-cluster inclusion probabilities were
then drawn from the prior Beta(alpha_between, beta_between), ignoring the
variable's edges. Variables updated later in the same sweep used these random
values to decide whether to join the new cluster.

Under the default sbm_prior() this biases the posterior over partitions
(cluster-count probabilities, co-clustering, clustering Bayes factors). With
strongly informative priors such as sbm_prior(8, 1, 1, 8) the bias is
negligible.

# Two triangles joined by one edge; large n pins the graph down.
p <- 6
edges <- rbind(c(1, 2), c(1, 3), c(2, 3), c(4, 5), c(4, 6), c(5, 6), c(3, 4))
omega <- diag(p)
omega[edges] <- -0.3
omega[edges[, 2:1]] <- -0.3
set.seed(20261023)
x <- matrix(rnorm(3000 * p), 3000, p) %*% chol(solve(omega))
fit <- bgm(x, variable_type = "continuous", edge_prior = sbm_prior(),
           iter = 100000, warmup = 2000, chains = 2, seed = 20261023)
alloc <- do.call(rbind, fit$raw_samples$allocations)
table(apply(alloc, 1, function(z) length(unique(z)))) / nrow(alloc)
# Before the fix: P(1 cluster) ~ 0.36; exact posterior (all 203 partitions) ~ 0.33

Proposed Changes / New Functionality

In the non-singleton branch of block_allocations_mfm_sbm
(src/priors/sbm_edge_prior.cpp), each between-cluster probability of the new
cluster is now drawn from Beta(alpha_between + s_ib, beta_between + n_b - s_ib).
Here s_ib is the number of edges from the variable to cluster b, and n_b is the
size of cluster b. The within-cluster entry keeps its prior draw, because the new
cluster has no within-cluster pairs yet.

This affects the sampling algorithm in src/ only. There are no changes to prior
specifications, summary outputs, or R functions.

Fixes #

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Performance optimization
  • Statistical / methodological update

Documentation and Release Notes

  • Updated function documentation for any signature or behaviour changes
  • Regenerated man/*.Rd files if roxygen comments changed
  • Added or updated NEWS.md entry if the change affects users

Testing and Validation

  • Compared the sampler with exact enumeration of all partitions (6 variables,
    averaged over the sampled graphs). Before the fix: total variation distance
    0.107, largest deviation 11.8 Monte Carlo SEs. After: 0.018 and 1.8 SEs, i.e.
    within Monte Carlo error. sbm_prior(8, 1, 1, 8) is unaffected (TV 0.0009).

  • Added tests/testthat/test-sbm-allocation.R (nightly tier,
    BGMS_RUN_SLOW_TESTS=true, ~8 s). It checks P(number of clusters) against
    exact enumeration within 0.02: the gap is 0.004 after the fix and 0.036 before,
    so the test fails on the old code.

  • Full test suite: 8,514 passed, 0 failed. The slow tiers of
    test-sbm-correction.R and test-mixed-correction.R also pass.

  • Added or updated tests in tests/testthat/ when needed

  • Ran the project styler

source("inst/styler/bgms_style.R")
styler::style_pkg(style = bgms_style)
  • Ran lintr::lint_package()
  • Ran roxygen2::roxygenise() if roxygen comments changed
  • Ran rcmdcheck::rcmdcheck(args = c("--no-manual", "--as-cran")) for non-trivial changes
  • Verified numerical behaviour where relevant

Additional Notes

  • Results under the default sbm_prior() will change numerically (partition
    probabilities, cluster counts, clustering Bayes factors). Results under strongly
    informative Beta priors are essentially unchanged.
  • The singleton branch is unchanged; it was already correct.
  • Follow-up: the "joint" branch (block_allocations_mfm_sbm_corrected) has the
    same prior draw. Its correct conditional includes the normalizer tilt, so it
    needs a separate fix.

This branch has not been deployed

No deployments
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