Skip to content

Added ability for priors of transformed distributions to have their p… #2551

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

Merged
merged 4 commits into from
Jul 25, 2024

Conversation

hvarfner
Copy link
Contributor

A suggested fix to #2550.

The proposed solution adds a _transformed_ buffer to all of the priors of TransformedDistributions (LogNormal, HalfNormal, HalfCauchy), as their original parameters cannot be saved to state_dict. Thus these parameters will be saved and loaded from state_dict, and sets the value of the parameters of the corresponding base distributions.

Added some tests for these methods as well, hopefully they are sufficient.

@Balandat
Copy link
Collaborator

This seems like a reasonable workaround to me for the time being, though we should ideally see if saving/loading state dicts can be better supported on the pytorch side.

@dme65 dme65 self-requested a review July 25, 2024 17:54
Copy link
Collaborator

@dme65 dme65 left a comment

Choose a reason for hiding this comment

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

This lgtm as a workaround for now, but I agree with @Balandat that we should try to address this on the PyTorch side in the long-term. Please take care of the linting before merging

@dme65 dme65 merged commit 917603c into cornellius-gp:main Jul 25, 2024
7 checks passed
gpleiss pushed a commit that referenced this pull request Aug 7, 2024
…rediction strategy (#2559)

* Added ability for priors of transformed distributions to have their p… (#2551) (#2552)

* avoid unnecessary memory allocation for downdate in SGPR prediction

---------

Co-authored-by: David Eriksson <[email protected]>
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.

3 participants