- Notifications
You must be signed in to change notification settings - Fork180
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Zuko density estimators#1088
Zuko density estimators#1088
Conversation
* CLN pyproject.toml* CLN optional deps comment* CLN alphabetical order
* fix x_o and broken link tutorial 7* typo in title* suppress plotting output---------Co-authored-by: Matthijs <matthijs@example.com>
The base branch of this PR issbi-dev:974-zuko-density-estimators-interface right now. You probably want to merge it into |
| @pytest.mark.parametrize("density_estimator", (NFlowsFlow, ZukoFlow)) | ||
| @pytest.mark.parametrize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Add here the new build functions, which are not yet covered :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
what do you exactly mean? I have a pytest function which does the following:
@pytest.mark.parametrize( "build_density_estimator", ( build_maf, build_maf_rqs, build_nsf, build_zuko_nice, build_zuko_maf, build_zuko_nsf, build_zuko_ncsf, build_zuko_sospf, build_zuko_naf, build_zuko_unaf, build_zuko_cnf, build_zuko_gf, build_zuko_bpf, ),)Or is that not enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hey, great work! So many new options.
I added a few comments, which should be resolved before merging into main.
As this PR wants to merge into another branch, I will already approve it.
Feel free to merge it yourself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
See comments by@manuelgloeckler et al. In the meantime, let's merge this into the the sbi-dev:974 branch, and create a new PR to merge that branch into main. In the meantime, could you close PRs#1068 and#1064, if you are no longer using them? I think they might be duplicates of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I copied my review from#1068. Those suggestions apply once you rebase/merge the currentmain into your branch. You can directly commit them with "Add suggestion to batch" and then "Commit suggestion".
Let me know if I should do this directly.
Co-authored-by: Sebastian Bischoff <sebastian@salzreute.de>
Co-authored-by: Sebastian Bischoff <sebastian@salzreute.de>
Co-authored-by: Sebastian Bischoff <sebastian@salzreute.de>
bf65102 intosbi-dev:974-zuko-density-estimators-interface* update zuko to 1.1.0* test zuko_gmm commit* build_zuko_nsf added* add build_zuko_naf, update test* add license change to pr template.* CLN pyproject.toml (#1009)* CLN pyproject.toml* CLN optional deps comment* CLN alphabetical order* fix x_o and broken link tutorial 7 (#1003)* fix x_o and broken link tutorial 7* typo in title* suppress plotting output---------Co-authored-by: Matthijs <matthijs@example.com>* replace prepare_for_sbi in tutorials (#1013)* add zuko density estimators* not working gmm* update tests for PR* update PR for pyright* resolve pyright* add reportArgumentType* resolve pyright issue* resolve all issues pyright* resolve pyright* add typing and docstring* add functions from factory to test* remove comment mdn file* add docstrings flow file* add docstring in density_estimator_test.py* Update sbi/neural_nets/flow.pyCo-authored-by: Sebastian Bischoff <sebastian@salzreute.de>* Update sbi/neural_nets/flow.pyCo-authored-by: Sebastian Bischoff <sebastian@salzreute.de>* Update sbi/neural_nets/flow.pyCo-authored-by: Sebastian Bischoff <sebastian@salzreute.de>* removed pyright---------Co-authored-by: bkmi <12955549+bkmi@users.noreply.github.com>Co-authored-by: Nastya Krouglova <nastyakrouglova@Nastyas-MacBook-Pro.local>Co-authored-by: Jan Boelts <jan.boelts@mailbox.org>Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>Co-authored-by: Matthijs Pals <34062419+Matthijspals@users.noreply.github.com>Co-authored-by: Matthijs <matthijs@example.com>Co-authored-by: zinaStef <49067201+zinaStef@users.noreply.github.com>Co-authored-by: Sebastian Bischoff <sebastian@salzreute.de>
* Zuko density estimators (#1088)* update zuko to 1.1.0* test zuko_gmm commit* build_zuko_nsf added* add build_zuko_naf, update test* add license change to pr template.* CLN pyproject.toml (#1009)* CLN pyproject.toml* CLN optional deps comment* CLN alphabetical order* fix x_o and broken link tutorial 7 (#1003)* fix x_o and broken link tutorial 7* typo in title* suppress plotting output---------Co-authored-by: Matthijs <matthijs@example.com>* replace prepare_for_sbi in tutorials (#1013)* add zuko density estimators* not working gmm* update tests for PR* update PR for pyright* resolve pyright* add reportArgumentType* resolve pyright issue* resolve all issues pyright* resolve pyright* add typing and docstring* add functions from factory to test* remove comment mdn file* add docstrings flow file* add docstring in density_estimator_test.py* Update sbi/neural_nets/flow.pyCo-authored-by: Sebastian Bischoff <sebastian@salzreute.de>* Update sbi/neural_nets/flow.pyCo-authored-by: Sebastian Bischoff <sebastian@salzreute.de>* Update sbi/neural_nets/flow.pyCo-authored-by: Sebastian Bischoff <sebastian@salzreute.de>* removed pyright---------Co-authored-by: bkmi <12955549+bkmi@users.noreply.github.com>Co-authored-by: Nastya Krouglova <nastyakrouglova@Nastyas-MacBook-Pro.local>Co-authored-by: Jan Boelts <jan.boelts@mailbox.org>Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>Co-authored-by: Matthijs Pals <34062419+Matthijspals@users.noreply.github.com>Co-authored-by: Matthijs <matthijs@example.com>Co-authored-by: zinaStef <49067201+zinaStef@users.noreply.github.com>Co-authored-by: Sebastian Bischoff <sebastian@salzreute.de>* merge* hate* merge* merge* merge* merge* MERGE* remove cnf* implement changes Jan* Update sbi/neural_nets/factory.pyCo-authored-by: Jan <janfb@users.noreply.github.com>* resolve issues Jan* undo changes to tutorials folder.* sort dependencies.---------Co-authored-by: bkmi <12955549+bkmi@users.noreply.github.com>Co-authored-by: Nastya Krouglova <nastyakrouglova@Nastyas-MacBook-Pro.local>Co-authored-by: Jan Boelts <jan.boelts@mailbox.org>Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>Co-authored-by: Matthijs Pals <34062419+Matthijspals@users.noreply.github.com>Co-authored-by: Matthijs <matthijs@example.com>Co-authored-by: zinaStef <49067201+zinaStef@users.noreply.github.com>Co-authored-by: Sebastian Bischoff <sebastian@salzreute.de>Co-authored-by: Jan <janfb@users.noreply.github.com>
What does this implement/fix? Explain your changes
We implemented the Zuko density estimators (i.e., NICE, (MAF), NSF, NCSF, SOSPF, NAF, UNAF, CNF, GF, BPF) described inhttps://github.com/probabilists/zuko/tree/master. The only flow that is not implemented yet, is theGaussian Mixture Model (GMM).
Does this close any currently open issues?
Fixes # (issue)
Fixes#974
However, the implementation of Zuko GMM is still required.
Any relevant code examples, logs, error output, etc?
Any other comments?
This pull request resolve proposals Guy on Zuko integration.
Checklist
Put an
xin the boxes that apply. You can also fill these out after creatingthe 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.
guidelines
with
pytest.mark.slow.guidelines
main(or there are no conflicts withmain)