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

Fixed a bug from CNRE #815

Merged
merged 4 commits into from
Mar 8, 2023
Merged

Fixed a bug from CNRE #815

merged 4 commits into from
Mar 8, 2023

Conversation

bkmi
Copy link
Contributor

@bkmi bkmi commented Mar 6, 2023

I noticed a bug in the code that implements contrastive neural ratio estimation. I fixed that here.

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, thanks for fixing this! I added two comments.
Did you experience any performance differences due to this bug?

sbi/inference/snre/snre_c.py Outdated Show resolved Hide resolved
sbi/inference/snre/snre_c.py Show resolved Hide resolved
@bkmi
Copy link
Contributor Author

bkmi commented Mar 6, 2023

Great, thanks for fixing this! I added two comments. Did you experience any performance differences due to this bug?

Yes, it will change the behavior of the algorithm. There was an extra (incorrect) K in the denominator.

@janfb
Copy link
Contributor

janfb commented Mar 6, 2023

Thanks for the clarifications. Tests are failing due to black formatting. Please make sure you have the latest v.23.1.0 black version installed and run it locally, e.g., $ black ..

@bkmi
Copy link
Contributor Author

bkmi commented Mar 8, 2023

Is it possible to rerun the test from github? I did black formatting but the test was canceled.

@janfb
Copy link
Contributor

janfb commented Mar 8, 2023

Yes. Not sure this works for you as well, but for me there is a "re-run jobs" button in the top right corner.

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 e3a041a into sbi-dev:main Mar 8, 2023
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