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

Removed the optional correction of the precision matrix #497

Merged
merged 1 commit into from
May 21, 2021
Merged

Removed the optional correction of the precision matrix #497

merged 1 commit into from
May 21, 2021

Conversation

famura
Copy link
Contributor

@famura famura commented May 20, 2021

I suggest to remove the optional correction of the precision matrix for 2 reasons:

  1. Forcefully making a matrix positive definite is hacky, and
  2. This option was only useful for the evil case where SNPE-A yields a denisty esimator with lower precision than the proposal prior. Hoewever, if this is the case, rejection sampling from MoG typically "fails" (super high rejection rates). Since MCMC sampling is disabled for SNPE-A (makes sense), I think it is best to let the attept fail and signal the user to try different params or a different random seed 🙃

@codecov-commenter
Copy link

Codecov Report

Merging #497 (907bf46) into main (3c3a79e) will increase coverage by 0.00%.
The diff coverage is 40.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #497   +/-   ##
=======================================
  Coverage   67.69%   67.70%           
=======================================
  Files          55       55           
  Lines        3969     3967    -2     
=======================================
- Hits         2687     2686    -1     
+ Misses       1282     1281    -1     
Flag Coverage Δ
unittests 67.70% <40.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sbi/inference/snpe/snpe_a.py 66.50% <40.00%> (+0.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c3a79e...907bf46. Read the comment docs.

@michaeldeistler michaeldeistler merged commit 8345563 into sbi-dev:main May 21, 2021
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.

3 participants