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

Add change base distribution example #34

Merged
merged 3 commits into from
Jun 2, 2023

Conversation

kazewong
Copy link
Contributor

Submitting this PR as part of JOSS review openjournals/joss-reviews#5361

One of the biggest benefit of this package from a user point of view is one can change the base distribution easily. I have tried doing that in other packages but it was pretty tedious in general, whereas in this package one can simply define the forward and log_prob function and be done with it. I think this is a nice feature to highlight in the repo (at least in word so the users are aware of that. The authors have already highlighted this in the draft but it would nice to see it on the repo since there is where most of the people will look first.)

I made a simple example script demonstrating this capability, by basically fetching the two moons example and training on it with a Gaussian mixture as the base distribution, which makes the training more stable and the final result nicer. Feel free to add more comments around it.

@VincentStimper VincentStimper self-requested a review June 1, 2023 07:58
Copy link
Owner

@VincentStimper VincentStimper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kazewong,

that's a great point and contribution! Could you please remove the generated output of the notebook to keep the repository size at bay and add a heading to the notebook? Then I'm happy to merge.

Best regards,
Vincent

@kazewong
Copy link
Contributor Author

kazewong commented Jun 1, 2023

@VincentStimper, I have removed the generated output and add heading.

@VincentStimper
Copy link
Owner

Great, thanks!

@VincentStimper VincentStimper merged commit a7221de into VincentStimper:master Jun 2, 2023
1 of 2 checks passed
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.

None yet

2 participants