Skip to content

Conversation

abelaba
Copy link
Collaborator

@abelaba abelaba commented Aug 8, 2025

This pull request builds upon #1557 and addresses remaining comments by updating DensityEstimatorBuilder Protocol to be more flexible. This change allows the protocol to work with different types of conditional density estimators.

Copy link

codecov bot commented Aug 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@67d7e1c). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1633   +/-   ##
=======================================
  Coverage        ?   82.56%           
=======================================
  Files           ?      135           
  Lines           ?    10957           
  Branches        ?        0           
=======================================
  Hits            ?     9047           
  Misses          ?     1910           
  Partials        ?        0           
Flag Coverage Δ
unittests 82.56% <100.00%> (?)

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

Files with missing lines Coverage Δ
sbi/inference/trainers/base.py 93.54% <100.00%> (ø)
sbi/inference/trainers/nle/mnle.py 95.65% <100.00%> (ø)
sbi/inference/trainers/nle/nle_a.py 100.00% <100.00%> (ø)
sbi/inference/trainers/nle/nle_base.py 97.82% <100.00%> (ø)
sbi/inference/trainers/npe/mnpe.py 95.65% <100.00%> (ø)
sbi/inference/trainers/npe/npe_a.py 63.91% <100.00%> (ø)
sbi/inference/trainers/npe/npe_b.py 100.00% <100.00%> (ø)
sbi/inference/trainers/npe/npe_base.py 94.77% <100.00%> (ø)
sbi/inference/trainers/npe/npe_c.py 73.33% <100.00%> (ø)
sbi/neural_nets/estimators/base.py 66.10% <100.00%> (ø)

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! A couple of comments and questions remain.

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.

some last comments, but almost done! 👍

@abelaba abelaba force-pushed the fix/protocol-for-estimator-builder branch from 13a416b to 8b41b1a Compare August 13, 2025 13:31
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.

Almost done, please just revert the changes done in sbi_types.py so that it in sync with main, i.e., does not show up in this PR anymore. thanks! 🙏

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.

All good now, well done @abelaba 👏

and thanks again to @ARna06 for starting this!

@janfb janfb merged commit 7064286 into sbi-dev:main Aug 14, 2025
9 checks passed
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