Skip to content

Conversation

schroedk
Copy link
Collaborator

@schroedk schroedk commented Aug 19, 2024

Fix type annotation in class 'ConditionedPotential'. Correct return type

of the function 'posterior_estimator_based_potential'

What does this implement/fix? Explain your changes

  • The argument potential_fn of the class ConditionedPotential must be of type BasePotential

Does this close any currently open issues?

Fixes #1055

Any relevant code examples, logs, error output, etc?

...

Any other comments?

...

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.

  • I have read and understood the contribution
    guidelines
  • I agree with re-licensing my contribution from AGPLv3 to Apache-2.0.
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have reported how long the new tests run and potentially marked them
    with pytest.mark.slow.
  • New and existing unit tests pass locally with my changes
  • I performed linting and formatting as described in the contribution
    guidelines
  • I rebased on main (or there are no conflicts with main)
  • For reviewer: The continuous deployment (CD) workflow are passing.

…rn type

of the function 'posterior_estimator_based_potential'
Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.90%. Comparing base (8ee2a2c) to head (33679f3).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1222      +/-   ##
==========================================
- Coverage   84.86%   76.90%   -7.96%     
==========================================
  Files          97      101       +4     
  Lines        7677     7941     +264     
==========================================
- Hits         6515     6107     -408     
- Misses       1162     1834     +672     
Flag Coverage Δ
unittests 76.90% <100.00%> (-7.96%) ⬇️

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

Files Coverage Δ
sbi/analysis/conditional_density.py 82.53% <100.00%> (-7.79%) ⬇️
.../inference/potentials/posterior_based_potential.py 95.34% <100.00%> (+0.11%) ⬆️
sbi/utils/conditional_density_utils.py 70.13% <100.00%> (-20.78%) ⬇️

... and 32 files with indirect coverage changes

@janfb janfb self-assigned this Aug 19, 2024
@janfb janfb added the bug Something isn't working label Aug 19, 2024
@janfb janfb added this to the Hackathon and release 0.23 milestone Aug 19, 2024
@schroedk schroedk requested a review from janfb August 19, 2024 10:13
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.

Thanks for fixing this 👍

It makes absolute sense to type as BasePotentials here because we are calling .device and .set_x below.

@janfb janfb merged commit 5de7784 into main Aug 19, 2024
6 checks passed
@janfb janfb deleted the refactor/1055-potential-func-typing branch August 19, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type 'potential_fn' properly, not 'Callable'
2 participants