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

60 add vi helper #63

Merged
merged 24 commits into from
Jan 18, 2023
Merged

Conversation

Tennessee-Wallaceh
Copy link
Collaborator

Initial solution to #60.

Key points:

  • Creates a new flowjax.train submodule, for organisational purposes. The old train utils are moved to flowjax.train.data_fit

  • New VI helper is in flowjax.train.variational_fit

  • Doesn't include a sample_and_log_prob method, which can come in separate work to keep the PRs simpler

  • I am currently adding some tests

@Tennessee-Wallaceh Tennessee-Wallaceh marked this pull request as draft January 16, 2023 11:54
@danielward27
Copy link
Owner

Nice, I can add in the sample_and_log_prob method.

@Tennessee-Wallaceh Tennessee-Wallaceh marked this pull request as ready for review January 16, 2023 16:43
@danielward27 danielward27 requested review from danielward27 and removed request for danielward27 January 16, 2023 17:05
Copy link
Owner

@danielward27 danielward27 left a comment

Choose a reason for hiding this comment

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

I've added some comments, thanks again for adding this :)

@danielward27
Copy link
Owner

I've added the sample_and_log_prob method in #65, so the training script should be updated to use that.

@danielward27
Copy link
Owner

By the way, I have no strong feelings about "train" vs "fit", but feel that we should be consistent and use one or the other and not both.

@Tennessee-Wallaceh
Copy link
Collaborator Author

By the way, I have no strong feelings about "train" vs "fit", but feel that we should be consistent and use one or the other and not both.

How about fit_to_data and fit_to_variational_target?

@danielward27 danielward27 merged commit 3f248cf into danielward27:main Jan 18, 2023
@Tennessee-Wallaceh Tennessee-Wallaceh deleted the 60-add-vi-helper branch May 17, 2023 15:18
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