Skip to content

Allow var_names to be propagated to nutpie sampler #7850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Jul 8, 2025
Merged

Allow var_names to be propagated to nutpie sampler #7850

merged 12 commits into from
Jul 8, 2025
+57 −7

Conversation

PabloRoque
Copy link
Contributor

@PabloRoque PabloRoque commented Jul 7, 2025 •
edited
Loading

Description

Passing var_names to nutpie sampler has been supported since 0.13.3.

This PR just activates this option when using nutpie sampler from pymc

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7850.org.readthedocs.build/en/7850/

@welcome Welcome
Copy link

welcome bot commented Jul 7, 2025

Thank You Banner]
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

@PabloRoque PabloRoque changed the title Allow pm.sample(...,var_names=var_names, nuts_sampler=nutpie) Allow pm.sample(...,var_names=var_names, nuts_sampler=nutpie) Jul 7, 2025
@PabloRoque PabloRoque changed the title Allow pm.sample(...,var_names=var_names, nuts_sampler=nutpie) Allow pm.sample(...,var_names=var_names, nuts_sampler="nutpie") Jul 7, 2025
ricardoV94
ricardoV94 reviewed Jul 7, 2025
View reviewed changes
tests/sampling/test_mcmc_external.py Outdated
@@ -16,20 +16,20 @@
import numpy.testing as npt
import pytest

from pymc import Data, Model, Normal, sample
import pymc as pm
Copy link
Member

Choose a reason for hiding this comment

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

Why this change? We prefer to test with explicit imports

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am used to this standard, but happy to change it back

Copy link
Member

Choose a reason for hiding this comment

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

I don't see where it say you shouldn't import objects? Anyway changing unrelated code / previous approach leads to noise in the git changes. If you would want to propose a new standard it would also be applied to the whole codebase, not the single file you happened to look at.

ricardoV94
ricardoV94 reviewed Jul 7, 2025
View reviewed changes
Copy link
Member

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

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

Looks good, just some nits on the tests

PabloRoque and others added 6 commits July 7, 2025 12:43
@PabloRoque
Explicitly importing objects from module
b020736
@PabloRoque
Add tune, draws, random_seed to kwargs
40e2b94
@PabloRoque @ricardoV94
Add progress_bar=False, compute_convergence_checks=False.
544dd56 
Co-authored-by: Ricardo Vieira <ricardo.vieira1994@gmail.com>
@PabloRoque @ricardoV94
Make use of xr.testing.assert_allclose
75e35de 
Co-authored-by: Ricardo Vieira <ricardo.vieira1994@gmail.com>
@PabloRoque @ricardoV94
Remove the last free_RV from sampling
225a584 
Co-authored-by: Ricardo Vieira <ricardo.vieira1994@gmail.com>
@PabloRoque
Move nuts_sampler to sample args
9465bd2
@PabloRoque PabloRoque requested a review from ricardoV94 July 7, 2025 12:13
ricardoV94
ricardoV94 reviewed Jul 8, 2025
View reviewed changes
PabloRoque and others added 2 commits July 8, 2025 11:30
@PabloRoque @ricardoV94
Update tests/sampling/test_mcmc_external.py
9a6e06e 
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@PabloRoque
Fix typo
07975e9
@ricardoV94 ricardoV94 changed the title Allow pm.sample(...,var_names=var_names, nuts_sampler="nutpie") Allow var_names to be propagated to nutpie sampler Jul 8, 2025
@codecov Codecov
Copy link

codecov bot commented Jul 8, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.92%. Comparing base (ded582e) to head (467b6a6).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7850      +/-   ##
==========================================
+ Coverage   91.84%   92.92%   +1.08%     
==========================================
  Files         107      107              
  Lines       18301    18299       -2     
==========================================
+ Hits        16808    17004     +196     
+ Misses       1493     1295     -198
Files with missing lines Coverage Δ
pymc/sampling/mcmc.py 91.36% <ø> (+0.15%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ricardoV94 ricardoV94 merged commit ae43026 into pymc-devs:main Jul 8, 2025
25 checks passed
@welcome Welcome
Copy link

welcome bot commented Jul 8, 2025

Congratulations Banner]
Congrats on merging your first pull request! 🎉 We here at PyMC are proud of you! 💖 Thank you so much for your contribution 🎁

@ricardoV94
Copy link
Member

Thanks @PabloRoque !

@PabloRoque
Copy link
Contributor Author

First PR! 🥳

Thank you! @ricardoV94

@PabloRoque PabloRoque deleted the nutpie-var_names branch July 8, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ricardoV94 ricardoV94 ricardoV94 approved these changes

Assignees
No one assigned
Labels
maintenance samplers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PabloRoque @ricardoV94