Skip to content
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.

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

Make default STEP_METHODS a list that can be modified #7231

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Mar 29, 2024

Description

Having as a tuple is a bit cumbersome and makes it seem like it should not be modified. However, until we have a more proper system (disptach based probably), this is the API for other packages (including BART, pymc-experimental) to define default samplers.

This PR makes it a list (and over-engineers a fixture for the test :D, happy to remove if you think it's silly)

Related Issue

  • Closes #
  • Related to #

Checklist

Type of change

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

@ricardoV94 ricardoV94 force-pushed the make_step_methods_list branch from 71b6569 to 68457f2 Compare April 5, 2024 07:35
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.29%. Comparing base (22e8f0b) to head (68457f2).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7231   +/-   ##
=======================================
  Coverage   92.29%   92.29%           
=======================================
  Files         101      101           
  Lines       16892    16892           
=======================================
  Hits        15590    15590           
  Misses       1302     1302           
Files Coverage Δ
pymc/step_methods/__init__.py 100.00% <100.00%> (ø)

Copy link
Member

@jessegrabowski jessegrabowski left a comment

Choose a reason for hiding this comment

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

Looks nice and simple. Trying to think about corner cases where a simple list wouldn't be good enough for holding the step methods. Does it matter if a user appends the same sampler to the list multiple times? Or is the ordering of the list important anywhere downstream?

@ricardoV94
Copy link
Member Author

ricardoV94 commented Apr 5, 2024

The order should not be important, except that for RNG reproducibility it always ends up being important. Repeated entries shouldn't be a problem

@ricardoV94 ricardoV94 merged commit 034b9a4 into pymc-devs:main Apr 5, 2024
23 checks passed
@ricardoV94 ricardoV94 deleted the make_step_methods_list branch April 5, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants