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

batch size for posterior rejection sampling. #340

Merged
merged 1 commit into from
Sep 23, 2020
Merged

Conversation

janfb
Copy link
Contributor

@janfb janfb commented Sep 23, 2020

This will hopefully speed up rejection sampling from the posterior in case of leakage.

I moved the sampling and sample rejection to a separate function to reduce code repetition.

@janfb janfb added the enhancement New feature or request label Sep 23, 2020
@janfb janfb self-assigned this Sep 23, 2020
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.

Thanks for tackling this! I saw too late that you had only commited and not yet requested review....so probably this is still WIP. Anyways, I had already written some comments, here they are ;)

sbi/utils/sbiutils.py Show resolved Hide resolved
sbi/utils/sbiutils.py Outdated Show resolved Hide resolved
sbi/utils/sbiutils.py Outdated Show resolved Hide resolved
@jan-matthis
Copy link
Contributor

Just for future reference, a quick test showing the speed-up that can be obtained by batch sampling:
Screenshot 2020-09-23 at 10 39 08

@janfb janfb force-pushed the rejection-sampling-speedup branch 2 times, most recently from 2233c14 to e3ac4e5 Compare September 23, 2020 10:53
@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2020

Codecov Report

Merging #340 into main will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #340      +/-   ##
==========================================
+ Coverage   73.62%   73.65%   +0.02%     
==========================================
  Files          48       48              
  Lines        2684     2687       +3     
==========================================
+ Hits         1976     1979       +3     
  Misses        708      708              
Flag Coverage Δ
#unittests 73.65% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
sbi/utils/sbiutils.py 82.40% <100.00%> (+0.50%) ⬆️
sbi/inference/snre/snre_base.py 95.78% <0.00%> (ø)
sbi/inference/posteriors/base_posterior.py 66.83% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b758022...d05cd57. Read the comment docs.

sbi/utils/sbiutils.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jan-matthis jan-matthis 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 to me!

@janfb janfb merged commit 8f49862 into main Sep 23, 2020
@janfb janfb deleted the rejection-sampling-speedup branch September 23, 2020 12:48
jan-matthis added a commit that referenced this pull request Sep 23, 2020
jan-matthis added a commit that referenced this pull request Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants