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

ScipyOptimizer #794

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

j-wilson
Copy link

Related issue(s)/PRs:

Resolves #793

Summary

This PR introduces a ScipyOptimizer class for GPFlow Modules that automatically extracts parameter bounds and passes them to gpflow.optimizers.Scipy. This fixes an issue where unconstrained_variables that are actually constrained under the prior cause model fitting to fail.

Additionally, this PR fixes a bug in resample_hyperparameters where prior_on was ignored.

Fully backwards compatible: no

There are edge-cases which may cause this code to fail. For example, Scipy may choose to filter out parameters that it deems inactive and, in so doing, cause the provided bounds to be misspecified (since it does not filter out the corresponding bounds).

PR checklist

  • The quality checks are all passing
  • The bug case / new feature is covered by tests
  • Any new features are well-documented (in docstrings or notebooks)

@j-wilson
Copy link
Author

j-wilson commented Nov 17, 2023

@hstojic Here is a quick PR related to #793. I've opted to upload this here to better illustrate the goal of these changes. Additional tests and docstrings are needed. Ultimately, we may want to separate out the fix for #793, since this can likely be merged as is.

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.

randomize_hyperparameters ignores prior_on
1 participant