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: add batch dim to inv transform, add test. #867

Merged
merged 3 commits into from
Aug 3, 2023

Conversation

janfb
Copy link
Contributor

@janfb janfb commented Aug 2, 2023

closes #841

as suggested in #650 we need a batch dim when calling the inverse transform that was constructed from a MultipleIndependent prior (because of the CatTransform).

Thus, the fix is to ensure the batch dim in the optimization routines for map.

@janfb janfb added the bug Something isn't working label Aug 2, 2023
@janfb janfb requested a review from michaeldeistler August 2, 2023 14:50
@janfb janfb self-assigned this Aug 2, 2023
@janfb janfb mentioned this pull request Aug 2, 2023
7 tasks
@janfb janfb force-pushed the fix-map-with-indenpendent-prior branch from 1dd1057 to e840cce Compare August 2, 2023 14:52
@janfb janfb force-pushed the fix-map-with-indenpendent-prior branch from e840cce to cac60c4 Compare August 2, 2023 14:56
@janfb janfb force-pushed the fix-map-with-indenpendent-prior branch from 10bf134 to 5aef90b Compare August 2, 2023 15:15
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #867 (5e3bd38) into main (8769e25) will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #867      +/-   ##
==========================================
+ Coverage   74.60%   74.65%   +0.04%     
==========================================
  Files          80       80              
  Lines        6242     6242              
==========================================
+ Hits         4657     4660       +3     
+ Misses       1585     1582       -3     
Flag Coverage Δ
unittests 74.65% <ø> (+0.04%) ⬆️

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

Files Changed Coverage Δ
sbi/utils/sbiutils.py 80.22% <ø> (ø)

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

LGTM, thanks!

tests/linearGaussian_snle_test.py Show resolved Hide resolved
@janfb janfb merged commit b0d7ddf into main Aug 3, 2023
@janfb janfb deleted the fix-map-with-indenpendent-prior branch August 3, 2023 19:37
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.

Maximum-a-posterior estimate given a MultipleIndependent prior and MCMC sampler
2 participants