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

update the default value of jitter to JITTER_DEFAULT #6055

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

danhphan
Copy link
Member

This PR makes a minor change in gp models by setting the default value of jitter to JITTER_DEFAULT, instead of 0.0.

This could reduce the number of errors (like positive definite errors) when calling conditional functions.

I am not sure, but I'm not aware if there is any cases that the default jitter of 0.0 will be useful for end users. Please confirm if I'm wrong. Thank you.

@codecov
Copy link

codecov bot commented Aug 18, 2022

Codecov Report

Merging #6055 (e5504ed) into main (52f8673) will increase coverage by 1.56%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6055      +/-   ##
==========================================
+ Coverage   87.52%   89.09%   +1.56%     
==========================================
  Files          72       72              
  Lines       12924    12924              
==========================================
+ Hits        11312    11514     +202     
+ Misses       1612     1410     -202     
Impacted Files Coverage Δ
pymc/gp/gp.py 93.18% <100.00%> (ø)
pymc/step_methods/hmc/base_hmc.py 90.55% <0.00%> (+0.78%) ⬆️
pymc/parallel_sampling.py 86.79% <0.00%> (+0.99%) ⬆️
pymc/sampling_jax.py 97.05% <0.00%> (+97.05%) ⬆️

@danhphan
Copy link
Member Author

Hi, not sure why pymc/tests/test_sampling_jax.py not passed. It seems running alright at my local machine.

@bwengals
Copy link
Contributor

Reran the other build that failed, it passes now. The readthedocs build is failing I think because of the same issue upstream with jax that has since been fixed. Is there a way to rerun it to verify? I don't see anything in this PR that might cause an issue with docs.

It looks possible but maybe the feature is too new. I may not see a rebuild button because I'm not an admin on RTD, maybe @OriolAbril can see a rebuild button, if you have a sec?

@OriolAbril
Copy link
Member

asked for a rebuild, if it doesn't work it probably needs a rebase

@bwengals
Copy link
Contributor

Thanks for running that @OriolAbril, looks like everythings green now!

@bwengals bwengals self-requested a review August 23, 2022 22:06
Copy link
Contributor

@bwengals bwengals left a comment

Choose a reason for hiding this comment

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

lgtm, and thanks for fixing this @danhphan this will prevent a lot of unnecessary errors for people trying to use this

@danhphan
Copy link
Member Author

Hi, thanks @bwengals and @OriolAbril :)

@michaelosthege michaelosthege merged commit ab4b537 into pymc-devs:main Aug 25, 2022
@danhphan
Copy link
Member Author

Hi, thanks @michaelosthege

@danhphan danhphan deleted the update_jitter_default branch August 28, 2022 06:27
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