Contrastive Neural Ratio Estimation #787

bkmi · 2022-11-22T14:01:05Z

Dear SBI,

I would like to add our algorithm NRE-C to the SNRE section. I created the following checklist which I will update as the draft nears completion.

Let me know if there's anything else I can do!
Ben

…earching

janfb · 2022-11-22T14:07:06Z

Dear @bkmi
that's great, thank you for contributing NRE-C!
At the moment, the tests are failing because of black. So please run black, isort locally and push again.

janfb · 2022-11-22T14:34:17Z

black and isort are passing now 👍
However, pyright is complaining about some type erros. You can see them locally by running pyright, or on github by expanding the test logs:

Run pyright sbi
[7](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:8)
Loading configuration file at /home/runner/work/sbi/sbi/pyrightconfig.json
[8](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:9)
Assuming Python platform Linux
[9](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:10)
Searching for source files
[10](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:11)
Found 85 source files
[11](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:12)
pyright 1.1.280
[12](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:13)
/home/runner/work/sbi/sbi/sbi/inference/snre/snre_base.py
[13](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:14)
  /home/runner/work/sbi/sbi/sbi/inference/snre/snre_base.py:242:56 - error: Argument expression after ** must be a mapping with a "str" key type (reportGeneralTypeIssues)
[14](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:15)
  /home/runner/work/sbi/sbi/sbi/inference/snre/snre_base.py:272:60 - error: Argument expression after ** must be a mapping with a "str" key type (reportGeneralTypeIssues)
[15](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:16)
/home/runner/work/sbi/sbi/sbi/inference/snre/snre_c.py
[16](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:17)
  /home/runner/work/sbi/sbi/sbi/inference/snre/snre_c.py:165:31 - error: "float" is not iterable
[17](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:18)
    "__iter__" method not defined (reportGeneralTypeIssues)
[18](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:19)
  /home/runner/work/sbi/sbi/sbi/inference/snre/snre_c.py:180:16 - error: Expression of type "tuple[float, float]" cannot be assigned to return type "float"
[19](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:20)
    "tuple[float, float]" is incompatible with "float" (reportGeneralTypeIssues)
[20](https://github.com/mackelab/sbi/actions/runs/3524072067/jobs/5909002539#step:7:21)
4 errors, 0 warnings, 0 informations

bkmi · 2022-11-22T14:57:31Z

The tests are extremely slow. Any hints here? Is there a way to only run the ratio-relevant ones?

I could run only the tests that I changed, but I did slightly alter the RatioEstimator in snre_base, so maybe I cannot get away with it.

janfb · 2022-11-22T15:36:51Z

yes, you can run specific tests like this (in the sbi folder):

pytest tests/linearGaussian_snre_test.py::test_function

or run all tests in that file (exlcuding marked gpu and slow tests):

pytest -m "not slow and not gpu" tests/linearGaussian_snre_test.py

bkmi · 2022-11-23T11:32:51Z

I found a bug in RatioEstimator while running the tests #788.

If you like my proposal I will include the fix in this PR.

bkmi · 2022-11-23T12:44:27Z

I found a bug in RatioEstimator while running the tests #788.

If you like my proposal I will include the fix in this PR.

We decided to make this a different PR.

codecov-commenter · 2022-11-23T13:26:47Z

Codecov Report

Merging #787 (4a535e5) into main (6cdf678) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #787      +/-   ##
==========================================
+ Coverage   74.62%   74.80%   +0.17%     
==========================================
  Files          79       80       +1     
  Lines        6144     6187      +43     
==========================================
+ Hits         4585     4628      +43     
  Misses       1559     1559

Flag	Coverage Δ
unittests	`74.80% <100.00%> (+0.17%)`	⬆️

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

Impacted Files	Coverage Δ
sbi/inference/posteriors/direct_posterior.py	`98.21% <ø> (ø)`
...inference/potentials/likelihood_based_potential.py	`100.00% <ø> (ø)`
.../inference/potentials/posterior_based_potential.py	`100.00% <ø> (ø)`
sbi/inference/potentials/ratio_based_potential.py	`100.00% <ø> (ø)`
sbi/inference/snre/snre_b.py	`100.00% <ø> (ø)`
sbi/utils/sbiutils.py	`79.76% <ø> (ø)`
sbi/inference/__init__.py	`100.00% <100.00%> (ø)`
sbi/inference/snre/__init__.py	`100.00% <100.00%> (ø)`
sbi/inference/snre/snre_base.py	`94.87% <100.00%> (ø)`
sbi/inference/snre/snre_c.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

bkmi · 2022-11-24T23:27:33Z

I think this pull request is ready to be reviewed!

janfb

thanks again for adding this to SBI!
looks great already, I added some minor comments. One general question I have concerns the sequential version of NRE_C. If it is not exact this should be clearly stated in the docstrings and we should add a warning or even an error when it is used in sequential mode, informing about the practical consequences.
But maybe I am missing something here, I am happy to discuss this in more detail.

EDIT: Slow and gpu tests are passing!

sbi/inference/potentials/ratio_based_potential.py

sbi/inference/snre/__init__.py

sbi/inference/snre/snre_b.py

sbi/inference/snre/snre_base.py

sbi/inference/snre/snre_c.py

bkmi · 2022-11-25T09:27:48Z

I will comment more carefully soon, but just to be clear... It is not that NRE-C is anymore inaccurate than any other sequential method, rather the guarantee that, at optimum, $r(x \mid \theta) = \frac{p(x \mid \theta)}{p(x)}$ does not hold. Instead, there would be a multiplicative term which depends on $x$.

In NRE-A this optimum holds (only in the first round). In NRE-B, this never holds. NRE-C is just like NRE-A in this regard. NRE-B is the default method but does not carry the warning.

We could add the warning to all NRE methods, but I don't think that is a good answer. After all, this does not affect posteriors but rather ways to test them. Sequential methods generally make diagnostics on the posterior more difficult (or impossible). If we added a warning here, it is my opinion that a general statement that sequential methods are not easily testable would be more appropriate and honest.

Perhaps the best answer would be for me to be very specific in the comment about what is meant regarding the sequential behavior of NRE-C, namely that an unknown multiplicative term in $x$ appears when doing sequential training?

janfb · 2022-11-25T09:57:20Z

I see, thanks for clarifying!
I agree, it would be good to point this out in the docstring of SNRE_C (and also update the ones for the other NRE methods), e.g., sth like that the likelihood-to-evidence ratio approximation is exact only in the first round, but that this does not affect the accuracy of the posterior samples, with a pointer to the paper for more details.

bkmi · 2022-12-07T17:58:52Z

Thanks for your patience, NeurIPS delayed me on this.

(There are still TODOs)

michaeldeistler · 2022-12-08T10:56:08Z

Jumping in here regarding warnings for unnormalized posteriors: the way this is currently handled is that the MCMCPosterior (regardless of whether it is created by NRE, NLE, or sequential methods) warns if one calls .log_prob() here. To avoid the warning, the user should use .potential() for MCMCPosteriors.

I suggest to describe in the docstring of NRE_C that its .potential() will be incouraged to be normalized, but to not issue further warnings.

michaeldeistler

Thanks a lot Ben! It looks great overall, I made just a few comments.

README.md

sbi/inference/snre/snre_c.py

tutorials/16_implemented_methods.ipynb

bkmi · 2022-12-10T15:39:44Z

Jumping in here regarding warnings for unnormalized posteriors: the way this is currently handled is that the MCMCPosterior (regardless of whether it is created by NRE, NLE, or sequential methods) warns if one calls .log_prob() here. To avoid the warning, the user should use .potential() for MCMCPosteriors.

I suggest to describe in the docstring of NRE_C that its .potential() will be incouraged to be normalized, but to not issue further warnings.

I'm going to go with this approach.

janfb

Looks great now, thanks a lot for adding SNRE_C and for the detailed discussion!

We merged the other PR with BNRE already, which might have caused merge conflicts with this branch, sorry!
Once those are resolved, this is good to into main as well!

sbi/inference/snre/snre_c.py

…idate the discussion of the effects on .potential to that function / description of nre-c.

bkmi · 2022-12-19T11:10:52Z

I clarified the effects and left it to .potential to do the heavy lifting of warning the user w.r.t. unnormalized ratios of densities.

I also resolved the merge conflicts.

Assuming the tests pass I'd say it's good to go~

janfb

Thanks for fixing the conflicts and the pyright problems with the python versions.
Looks great now!

bkmi · 2022-12-19T16:45:33Z

Thanks everyone! Really appreciate the inclusion of NRE-C in sbi and I found the process to be enjoyable and collaborative!

michaeldeistler · 2022-12-20T08:11:46Z

Thanks a lot for contributing, Ben! And congrats on the work itself!

janfb · 2022-12-20T08:16:46Z

Yeah, thanks a lot @bkmi! It was a pleasure, and kudos for your work!

bkmi added 8 commits November 22, 2022 12:57

added support for enable transform

5e2a506

cast int | none to int

a35517c

pyright

102d83e

added the basics of snre_c, all todos are listed as TODO SNRE_C for s…

15c828e

…earching

finished some more, still got TODO SNRE_C

cdac5b0

wip

2a56da3

wrote snre_c?

655f55f

updated tests, readme, and snre base

8e22e00

bkmi mentioned this pull request Nov 22, 2022

Balanced neural ratio estimation #779

Merged

black and isort

7fd4b14

bkmi added 3 commits November 22, 2022 15:35

pyright

ad63d50

pyright attempt 2

b13d8e2

pyright 3rd time is a charm

1b6866a

bkmi mentioned this pull request Nov 23, 2022

Bug in test_embedding_nets_integration_training_device #788

Closed

bkmi added 2 commits November 23, 2022 12:34

added landing page, tutorial, fixed bug in exclude_invalid_x

2fe7173

removed the itertools product when it was unecessary

b24f1f7

bkmi marked this pull request as ready for review November 24, 2022 23:27

janfb requested a review from michaeldeistler November 25, 2022 08:20

janfb reviewed Nov 25, 2022

View reviewed changes

more names

5692db1

resolved some of the comments from review

8232103

michaeldeistler reviewed Dec 8, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

sbi/inference/snre/snre_c.py Outdated Show resolved Hide resolved

sbi/inference/snre/snre_c.py Show resolved Hide resolved

tutorials/16_implemented_methods.ipynb Show resolved Hide resolved

updated everything but the warning about potential

c804eb5

janfb reviewed Dec 16, 2022

View reviewed changes

sbi/inference/snre/snre_c.py Show resolved Hide resolved

bkmi added 2 commits December 19, 2022 11:51

updated to articulate the effects of nre-c more accurately and consol…

b5ca2ad

…idate the discussion of the effects on .potential to that function / description of nre-c.

solved conflicts

2b6f050

bkmi added 2 commits December 19, 2022 14:09

Merge branch 'main' into snre_c

8cd326c

changed to be compatible with lower versions of python

4a535e5

janfb approved these changes Dec 19, 2022

View reviewed changes

michaeldeistler merged commit 0ae6126 into sbi-dev:main Dec 19, 2022

bkmi deleted the snre_c branch December 19, 2022 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contrastive Neural Ratio Estimation #787

Contrastive Neural Ratio Estimation #787

bkmi commented Nov 22, 2022 •

edited

Loading

janfb commented Nov 22, 2022

janfb commented Nov 22, 2022 •

edited

Loading

bkmi commented Nov 22, 2022

janfb commented Nov 22, 2022

bkmi commented Nov 23, 2022

bkmi commented Nov 23, 2022

codecov-commenter commented Nov 23, 2022 •

edited

Loading

bkmi commented Nov 24, 2022

janfb left a comment •

edited

Loading

bkmi commented Nov 25, 2022 •

edited

Loading

janfb commented Nov 25, 2022

bkmi commented Dec 7, 2022 •

edited

Loading

michaeldeistler commented Dec 8, 2022

michaeldeistler left a comment

bkmi commented Dec 10, 2022

janfb left a comment

bkmi commented Dec 19, 2022

janfb left a comment

bkmi commented Dec 19, 2022

michaeldeistler commented Dec 20, 2022

janfb commented Dec 20, 2022

Contrastive Neural Ratio Estimation #787

Contrastive Neural Ratio Estimation #787

Conversation

bkmi commented Nov 22, 2022 • edited Loading

janfb commented Nov 22, 2022

janfb commented Nov 22, 2022 • edited Loading

bkmi commented Nov 22, 2022

janfb commented Nov 22, 2022

bkmi commented Nov 23, 2022

bkmi commented Nov 23, 2022

codecov-commenter commented Nov 23, 2022 • edited Loading

Codecov Report

bkmi commented Nov 24, 2022

janfb left a comment • edited Loading

Choose a reason for hiding this comment

bkmi commented Nov 25, 2022 • edited Loading

janfb commented Nov 25, 2022

bkmi commented Dec 7, 2022 • edited Loading

michaeldeistler commented Dec 8, 2022

michaeldeistler left a comment

Choose a reason for hiding this comment

bkmi commented Dec 10, 2022

janfb left a comment

Choose a reason for hiding this comment

bkmi commented Dec 19, 2022

janfb left a comment

Choose a reason for hiding this comment

bkmi commented Dec 19, 2022

michaeldeistler commented Dec 20, 2022

janfb commented Dec 20, 2022

bkmi commented Nov 22, 2022 •

edited

Loading

janfb commented Nov 22, 2022 •

edited

Loading

codecov-commenter commented Nov 23, 2022 •

edited

Loading

janfb left a comment •

edited

Loading

bkmi commented Nov 25, 2022 •

edited

Loading

bkmi commented Dec 7, 2022 •

edited

Loading