Skip to content

Fix stale trace count in test_mcmc_parallel_chain and run it in CI#2223

Merged
fehiepsi merged 2 commits into
pyro-ppl:masterfrom
esennesh:develop
Jul 20, 2026
Merged

Fix stale trace count in test_mcmc_parallel_chain and run it in CI#2223
fehiepsi merged 2 commits into
pyro-ppl:masterfrom
esennesh:develop

Conversation

@esennesh

Copy link
Copy Markdown
Contributor

test/test_compile.py::test_mcmc_parallel_chain[True] fails with assert 5 == 4 on any host with jax.device_count() >= 2.

#1910 merged postprocess_fn into the fori_collect loop, which adds two extra model traces when the model has deterministic sites (transform the init state + transform states during the loop). That PR updated test_mcmc_one_chain's expected count from 4 to 5 accordingly, but test_mcmc_parallel_chain was left asserting 4 — and the parallel-chain (pmap) path has the same two extra calls, so the model is now traced 5 times there too.

This went unnoticed for so long because CI never actually runs the test: test/test_compile.py is only executed in the batch without XLA_FLAGS="--xla_force_host_platform_device_count=2", so jax.device_count() is 1 and the skipif marker always skips it.

Bisection confirms this is independent of the JAX version: the test passes at 0e7bd20~1 and fails at 0e7bd20 (#1910) with the same jax 0.4.35, and current master fails identically on jax 0.7.2 through 0.10.2.

Changes made

  • Update test_mcmc_parallel_chain's deterministic trace-count assertion from 4 to 5, with the same explanatory comment Merge postprocess_fn into the fori_collect loop #1910 added to test_mcmc_one_chain.
  • Add test/test_compile.py -k "chain" to the multi-device "Test chains" CI batch so the test actually runs in CI instead of being silently skipped.

Links to related issues/PRs

Tests

  • XLA_FLAGS="--xla_force_host_platform_device_count=2" pytest -vs test/test_compile.py -k "chain" — 6 passed (previously test_mcmc_parallel_chain[True] failed with assert 5 == 4).
  • pytest test/test_compile.py (single device) — 6 passed, 2 skipped, unchanged.

Dependencies

  • None.

🤖 Generated with Claude Code

PR pyro-ppl#1910 merged postprocess_fn into the fori_collect loop, adding two
extra model traces for deterministic values, and updated
test_mcmc_one_chain accordingly (4 -> 5) but left test_mcmc_parallel_chain
asserting 4. The parallel-chain path has the same two extra calls, so the
test fails with `assert 5 == 4` on any host with two or more devices.

This went unnoticed because CI only runs test/test_compile.py without
XLA_FLAGS=--xla_force_host_platform_device_count=2, so the skipif marker
always skips the parallel-chain test. Update the assertion to 5 and add
the file to the multi-device "Test chains" batch so it actually runs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@fehiepsi fehiepsi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Eli!

…t` passing

Signed-off-by: Eli Sennesh <elisennesh@astera.org>
@fehiepsi
fehiepsi merged commit ecdd495 into pyro-ppl:master Jul 20, 2026
9 checks passed
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.

2 participants