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

Fix npe iid handling #1262

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Fix npe iid handling #1262

merged 2 commits into from
Sep 5, 2024

Conversation

janfb
Copy link
Contributor

@janfb janfb commented Sep 4, 2024

fixes #1256

Note: I removed warn_on_batched_x because it was shown whenever a batched x is passed, i.e., even when explicitly calling sample_batched. I think we do not need to issue the warning when calling the batched methods because users explicitly call those methods for the batched case.

For the iid case, in NPE, the user has to use a corresponding embedding net anyways, so this warning is also not helpful.

For the iid base in likelihood-based methods, it is natural that we can sample iid data, so also no need to issue the warning.

@janfb janfb force-pushed the fix-npe-iid-handling branch from 6406139 to 5b4c7ee Compare September 4, 2024 08:52
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.30%. Comparing base (8afd985) to head (5b4c7ee).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
sbi/inference/posteriors/direct_posterior.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1262      +/-   ##
==========================================
- Coverage   86.05%   78.30%   -7.76%     
==========================================
  Files         118      119       +1     
  Lines        8672     8696      +24     
==========================================
- Hits         7463     6809     -654     
- Misses       1209     1887     +678     
Flag Coverage Δ
unittests 78.30% <80.00%> (-7.76%) ⬇️

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

Files with missing lines Coverage Δ
sbi/utils/sbiutils.py 78.11% <ø> (-8.83%) ⬇️
sbi/utils/user_input_checks.py 76.31% <100.00%> (-0.13%) ⬇️
sbi/inference/posteriors/direct_posterior.py 97.67% <75.00%> (-1.13%) ⬇️

... and 27 files with indirect coverage changes

Copy link
Contributor

@michaeldeistler michaeldeistler left a comment

Choose a reason for hiding this comment

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

Perfect, thanks a ton!

@janfb janfb merged commit c33a855 into main Sep 5, 2024
6 checks passed
@janfb janfb deleted the fix-npe-iid-handling branch September 5, 2024 11:06
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.

NPE raises warning not error when conditioned on batched x without embedding net.
2 participants