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

Add QuadPotentialFullAdapt in pm.sample init #3858

Merged
merged 1 commit into from
Mar 28, 2020

Conversation

techytushar
Copy link
Contributor

Fixes #3845
@ColCarroll @twiecki @eigenfoo I have tried to add QuadPotentialFullAdapt to the pm.sample. Since I have no experience with this type of sampling, I am not sure whether the initialized mean and covariance are correct. It would be great if you could guide me a little, I'll update the PR with correct initializations.
Do I also need to add tests for this?

@twiecki twiecki requested a review from eigenfoo March 27, 2020 18:41
@codecov
Copy link

codecov bot commented Mar 27, 2020

Codecov Report

Merging #3858 into master will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3858      +/-   ##
==========================================
- Coverage   90.75%   90.73%   -0.03%     
==========================================
  Files         135      135              
  Lines       21184    21189       +5     
==========================================
  Hits        19226    19226              
- Misses       1958     1963       +5     
Impacted Files Coverage Δ
pymc3/sampling.py 84.62% <0.00%> (-0.55%) ⬇️

Copy link
Member

@eigenfoo eigenfoo left a comment

Choose a reason for hiding this comment

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

This looks great @techytushar! I've left just one comment below.

It would also be great if you could add a note to the release notes detailing this change.

Once that's fixed, I'm happy to merge!

pymc3/sampling.py Outdated Show resolved Hide resolved
Copy link
Member

@eigenfoo eigenfoo left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @techytushar!

@eigenfoo eigenfoo merged commit 7874295 into pymc-devs:master Mar 28, 2020
@twiecki
Copy link
Member

twiecki commented Mar 28, 2020

Thanks for the contribution @techytushar!

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.

Add full adaptation as option to pm.sample
3 participants