-
-
Notifications
You must be signed in to change notification settings - Fork 127
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
Convenient function to access inference methods and kwargs #795
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Note https://github.com/jax-ml/bayeux/blob/main/bayeux/_src/shared.py#L115 is what I use in If you run the slightly modified
on
|
As far as I know the signature for |
I am not so sure tests should be added for this? The Then, for |
@GStechschulte I see what you mean. I don't have a strong opinion here. The only thing I can add is that if we leave it untested it'll decrease coverage. I know high coverage does not mean our test suite is perfect, but I do think that in general lower coverage is worse. We could omit the Another option would be to merge as it is and open an issue so someone tests this in the future (as it's not critical). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mostly reviewed just the bayeux code, which LGTM!
Many thanks for the review @ColCarroll and @tomicapretto I added a small test to check that the keys ( |
Closes #791. This PR adds a convenient class
InferenceMethods
that allows users to access the available inference methods and kwargs.For example, the inference methods
and the default kwargs for a given inference method
Additionally, this convenience class is now imported and used in
backend/pymc.py
to obtain the bayeux and pymc inference methods. I have updated relevant doc strings and the alternative samplers notebook as well.