Skip to content

Refactor backend split: unify pymc and numpyro simulation loops - #60

Merged
aloctavodia merged 1 commit into
arviz-devs:mainfrom
ciguaran:ciguaran/backend_split
Jul 13, 2026
Merged

Refactor backend split: unify pymc and numpyro simulation loops#60
aloctavodia merged 1 commit into
arviz-devs:mainfrom
ciguaran:ciguaran/backend_split

Conversation

@ciguaran

Copy link
Copy Markdown
Contributor

In main, SBC class has three coupled concepts: the SBC loop itself, PyMC-specific sampling and model manipulation details and Numpyro-specifics.

This PR:
decouples SBC loop from PyMC from Numpyro. As a result, now there's a single simulation loop, backend-agnostic.

Consolidate the per-backend simulation loops into a single shared loop,
move pymc prior/posterior predictive into the adapter, apply a strategy
pattern for rank calculation, and clean up typing, logging, and ruff issues.

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

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.02985% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.87%. Comparing base (08ba511) to head (d93694c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
simuk/backend_adapter.py 70.96% 9 Missing ⚠️
simuk/pymc_adapter.py 97.39% 3 Missing ⚠️
simuk/numpyro_adapter.py 97.40% 2 Missing ⚠️
simuk/sbc.py 94.28% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
- Coverage   96.69%   95.87%   -0.82%     
==========================================
  Files           4        7       +3     
  Lines         545      631      +86     
==========================================
+ Hits          527      605      +78     
- Misses         18       26       +8     

☔ 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.

@aloctavodia aloctavodia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is great! I am leaving a couple of comments for future reference.

Comment thread simuk/pymc_adapter.py
Comment on lines +26 to +27
thinned_idata = self.trace.isel(draw=draw_indices)
posterior = extract(thinned_idata, group="posterior", keep_dataset=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not really related to this PR, but leaving the comment before I forget. Do we want to do this, or do we want to use https://python.arviz.org/projects/stats/en/stable/api/generated/arviz_stats.thin.html, which by default thins based on the ESS?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if we are to do this, we should do this for all other instances of sampling?

Comment thread simuk/pymc_adapter.py

return posterior, posterior_pred

def compute_single_rank(self, transform, name, posterior, simulation_idx, ref_params):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Again, for the future. We may want to do this directly using xarray

@aloctavodia
aloctavodia merged commit 912d019 into arviz-devs:main Jul 13, 2026
4 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.

4 participants