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

Introduce sample_with="direct" for SNPE #942

Merged
merged 3 commits into from
Feb 23, 2024
Merged

Introduce sample_with="direct" for SNPE #942

merged 3 commits into from
Feb 23, 2024

Conversation

michaeldeistler
Copy link
Contributor

@michaeldeistler michaeldeistler commented Feb 14, 2024

API change: For SNPE, one now has to use direct instead of rejection if one wants to directly sample from the density estimator. direct is also the new default.

Closes #918

@michaeldeistler michaeldeistler changed the title [WIP] Introduce sample_with="direct" for SNPE Introduce sample_with="direct" for SNPE Feb 14, 2024
Copy link

codecov bot commented Feb 14, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 76.00%. Comparing base (b856c07) to head (63386c1).

Files Patch % Lines
sbi/inference/snpe/snpe_base.py 33.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #942      +/-   ##
==========================================
- Coverage   76.04%   76.00%   -0.04%     
==========================================
  Files          80       80              
  Lines        6329     6331       +2     
==========================================
- Hits         4813     4812       -1     
- Misses       1516     1519       +3     
Flag Coverage Δ
unittests 76.00% <33.33%> (-0.04%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@janfb janfb left a comment

Choose a reason for hiding this comment

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

looks good, thanks for fixing this 🥇

todo: tests are failing because of black

sbi/inference/snpe/snpe_base.py Show resolved Hide resolved
sbi/inference/snpe/snpe_base.py Show resolved Hide resolved
@janfb janfb merged commit 116f99a into main Feb 23, 2024
1 of 3 checks passed
@janfb janfb deleted the direct branch February 23, 2024 10:22
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.

SNPE and SNLE build_posterior handle sample_with=rejection arg differently.
2 participants