Skip to content

Commit 36ca5db

Browse files
Fix SMCk common-ancestor walk crash when random_mass is zero
In msp_smc_k_common_ancestor_event, gsl_ran_flat occasionally draws random_mass == 0 (probability ~1/2^32 per CA event). With random_mass of zero, fenwick_find returns the first non-zero hull and remaining_mass = cum_sum - 0 lands at exactly x_hull->count. The walk loop with predicate `remaining_mass >= 0` then over-iterates by one match, runs off the head of the AVL tree, and trips the tsk_bug_assert at line 7694. Clamp remaining_mass strictly below x_hull->count so the loop terminates after exactly `count` matches. The fix is local; it does not change the RNG draw count or pattern, so seeded simulations that did not previously trip the bug produce identical trajectories. Add a regression test (test_smc_k_bug_repro) that replays the user-reported failure (50 diploid samples, seq_length 2e7, recomb 1e-8, demography change at t=30, seed 3940783591). Pre-fix the test aborts after ~74821 events; post-fix it completes in about a second.
1 parent 1b14a8a commit 36ca5db

2 files changed

Lines changed: 69 additions & 0 deletions

File tree

lib/msprime.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7688,6 +7688,18 @@ msp_smc_k_common_ancestor_event(
76887688
x_hull = msp_get_hull(self, hull_id, label);
76897689
remaining_mass = fenwick_get_cumulative_sum(coal_mass_index, hull_id) - random_mass;
76907690

7691+
/* When gsl_ran_flat draws random_mass == 0 (probability ~1/2^32 per
7692+
* call) — or when the zero-skip path in fenwick_find lands the result
7693+
* on a leading run of zero-valued slots — remaining_mass starts at
7694+
* exactly x_hull->count. The walk loop below decrements once per
7695+
* matching predecessor and exits on remaining_mass < 0; with
7696+
* remaining_mass == count it would over-iterate by one and run off
7697+
* the head of the AVL tree. Clamp here so the loop terminates after
7698+
* `count` matches. */
7699+
if (remaining_mass >= (double) x_hull->count) {
7700+
remaining_mass = (double) x_hull->count - 0.5;
7701+
}
7702+
76917703
/* find second hull */
76927704
search = &x_hull->left_avl_node;
76937705
for (search = search->prev; remaining_mass >= 0; search = search->prev) {

lib/tests/test_ancestry.c

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4656,6 +4656,62 @@ test_smc_k_gc(void)
46564656
}
46574657
}
46584658

4659+
/* Regression test: with this seed and parameters the SMCk common-ancestor
4660+
* walk used to abort via tsk_bug_assert at lib/msprime.c when gsl_ran_flat
4661+
* drew random_mass == 0 and the resulting remaining_mass equalled
4662+
* x_hull->count exactly, causing the walk to over-iterate by one and run
4663+
* off the head of the AVL tree. The fix clamps remaining_mass below count. */
4664+
static void
4665+
test_smc_k_bug_repro(void)
4666+
{
4667+
int ret;
4668+
uint32_t n = 100; /* 50 diploid individuals = 100 sample nodes */
4669+
sample_t *samples = malloc(n * sizeof(sample_t));
4670+
msp_t msp;
4671+
gsl_rng *rng = safe_rng_alloc();
4672+
tsk_table_collection_t tables;
4673+
4674+
CU_ASSERT_FATAL(samples != NULL);
4675+
memset(samples, 0, n * sizeof(sample_t));
4676+
/* All samples in pop 0 at time 0 (already zeroed). */
4677+
4678+
gsl_rng_set(rng, 3940783591UL);
4679+
4680+
/* sequence_length = 2e7, num_populations = 1 */
4681+
ret = build_sim(&msp, &tables, rng, 2e7, 1, samples, n);
4682+
CU_ASSERT_EQUAL_FATAL(ret, 0);
4683+
4684+
ret = msp_set_recombination_rate(&msp, 1e-8);
4685+
CU_ASSERT_EQUAL_FATAL(ret, 0);
4686+
4687+
/* pop 0: initial_size=3600, growth_rate=-0.03 */
4688+
ret = msp_set_population_configuration(&msp, 0, 3600.0, -0.03, true);
4689+
CU_ASSERT_EQUAL_FATAL(ret, 0);
4690+
4691+
/* At t=30, change pop 0 to initial_size=10000, growth=0 */
4692+
ret = msp_add_population_parameters_change(&msp, 30.0, 0, 10000.0, 0.0);
4693+
CU_ASSERT_EQUAL_FATAL(ret, 0);
4694+
4695+
/* SMC_K with hull_offset=1 (k=1) */
4696+
ret = msp_set_simulation_model_smc_k(&msp, 1.0);
4697+
CU_ASSERT_EQUAL_FATAL(ret, 0);
4698+
4699+
ret = msp_initialise(&msp);
4700+
CU_ASSERT_EQUAL_FATAL(ret, 0);
4701+
4702+
ret = msp_run(&msp, DBL_MAX, ULONG_MAX);
4703+
CU_ASSERT_EQUAL(ret, 0);
4704+
CU_ASSERT_TRUE(msp_is_completed(&msp));
4705+
4706+
msp_verify(&msp, 0);
4707+
4708+
ret = msp_free(&msp);
4709+
CU_ASSERT_EQUAL(ret, 0);
4710+
gsl_rng_free(rng);
4711+
free(samples);
4712+
tsk_table_collection_free(&tables);
4713+
}
4714+
46594715
int
46604716
main(int argc, char **argv)
46614717
{
@@ -4771,6 +4827,7 @@ main(int argc, char **argv)
47714827
{ "test_mixed_model_smc_k_large", test_mixed_model_smc_k_large },
47724828
{ "test_fenwick_rebuild_smc_k", test_fenwick_rebuild_smc_k },
47734829
{ "test_smc_k_gc", test_smc_k_gc },
4830+
{ "test_smc_k_bug_repro", test_smc_k_bug_repro },
47744831
CU_TEST_INFO_NULL,
47754832
};
47764833

0 commit comments

Comments
 (0)