Skip to content

Conversation

abelaba
Copy link
Collaborator

@abelaba abelaba commented Jul 29, 2025

This PR adds deprecation warnings for the stringly-typed build_posterior method parameters in all trainer classes and warns users to use the posterior_parameters parameter to pass posterior parameters instead of using the dictionary configurations. In addition, all the tutorials and tests have been updated to use the strongly typed dataclasses introduced in this PR. It also includes a how-to guide for using the PosteriorParameters data classes.

Copy link

codecov bot commented Jul 29, 2025

Codecov Report

❌ Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@a406410). Learn more about missing BASE report.
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
sbi/inference/trainers/base.py 97.61% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1627   +/-   ##
=======================================
  Coverage        ?   82.55%           
=======================================
  Files           ?      135           
  Lines           ?    10948           
  Branches        ?        0           
=======================================
  Hits            ?     9038           
  Misses          ?     1910           
  Partials        ?        0           
Flag Coverage Δ
unittests 82.55% <97.61%> (?)

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% <97.61%> (ø)

@abelaba abelaba changed the title Posterior parameters deprecation Deprecation Warnings for build_posterior stringly typed parameters Jul 29, 2025
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.

Great work! 👏
added a couple of comments and suggestion that should be easy to address.

Comment on lines 103 to 107
self.build_posterior,
locals(),
{
"vectorfield_sampling_parameters",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting is a bit verbose here, can we reduce the number of lines by removing the trailing commas? If no, it's fine.

@abelaba
Copy link
Collaborator Author

abelaba commented Aug 4, 2025

I have updated the previous implementation for raising deprecation warnings. Instead of duplicating the warning checks across each trainer subclass, I have centralized the check in the base class.

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.

Good refactoring! added a couple of last comments 👍

abelaba and others added 3 commits August 6, 2025 15:42
Co-authored-by: Jan <janfb@users.noreply.github.com>
Co-authored-by: Jan <janfb@users.noreply.github.com>
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 all good now - thanks! 🙏

@janfb janfb merged commit ebb8c6f into sbi-dev:main Aug 7, 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