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

Refactor diagnostics #1196

Merged
merged 9 commits into from
Aug 2, 2024
Merged

Refactor diagnostics #1196

merged 9 commits into from
Aug 2, 2024

Conversation

janfb
Copy link
Contributor

@janfb janfb commented Jul 21, 2024

What does this implement/fix? Explain your changes

  • added a separate function for generating posterior samples for a batch of observations (using batching where possible)
  • refactor SBC to outsource posterior sampling to that function
  • refactor TARP to be similar to SBC
  • adapt tutorial

run_sbc or run_tarp will do everything for the user, including posterior sampling. But now it is possible to first obtain posteroir samples and then pass them to _run_sbc and then to _run_tarp, without having to generate them again.

@janfb janfb self-assigned this Jul 21, 2024
@janfb janfb requested a review from michaeldeistler July 21, 2024 16:10
@janfb janfb force-pushed the refactor-diagnostics branch from 59c0c70 to 6829e2a Compare July 21, 2024 16:34
Copy link

codecov bot commented Jul 21, 2024

Codecov Report

Attention: Patch coverage is 88.23529% with 8 lines in your changes missing coverage. Please review.

Project coverage is 75.85%. Comparing base (0bdb9c5) to head (66e3b83).

Files Patch % Lines
sbi/diagnostics/tarp.py 75.86% 7 Missing ⚠️
sbi/diagnostics/sbc.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1196      +/-   ##
==========================================
- Coverage   84.35%   75.85%   -8.51%     
==========================================
  Files          96       97       +1     
  Lines        7702     7673      -29     
==========================================
- Hits         6497     5820     -677     
- Misses       1205     1853     +648     
Flag Coverage Δ
unittests 75.85% <88.23%> (-8.51%) ⬇️

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

Files Coverage Δ
sbi/diagnostics/__init__.py 100.00% <100.00%> (ø)
sbi/utils/diagnostics_utils.py 100.00% <100.00%> (ø)
sbi/diagnostics/sbc.py 95.77% <94.44%> (+0.42%) ⬆️
sbi/diagnostics/tarp.py 67.85% <75.86%> (-10.17%) ⬇️

... and 23 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.

This has a lot of overlap with the PR with the test for MDN calibration, which one should I review?

@janfb
Copy link
Contributor Author

janfb commented Jul 30, 2024

This has a lot of overlap with the PR with the test for MDN calibration, which one should I review?

this one please. The MDN only adds the test, but copied all the changes from here.

@janfb janfb force-pushed the refactor-diagnostics branch 2 times, most recently from e5938ff to 931892f Compare August 2, 2024 07:30
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.

Awesome! A few comments below.

@janfb janfb force-pushed the refactor-diagnostics branch from 931892f to 84e3f3d Compare August 2, 2024 09:40
@janfb janfb force-pushed the refactor-diagnostics branch from 8a7f4b0 to 66e3b83 Compare August 2, 2024 11:03
@janfb janfb merged commit 2fd89a8 into main Aug 2, 2024
6 checks passed
@janfb janfb deleted the refactor-diagnostics branch August 2, 2024 11:30
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.

2 participants