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

Handle unconstrained priors in randomize_hyperparameters #796

Merged
merged 4 commits into from
Nov 27, 2023

Conversation

uri-granta
Copy link
Collaborator

Related issue(s)/PRs: #793

Summary

Fix randomize_hyperparameters to handle unconstrained priors.

Also change the logic for handling Sigmoid constraints: we now prioritise sampling from the prior if there is one, and sample from the constrained domain only if there isn't.

Fully backwards compatible: no

New Sigmoid behaviour is more logical and flexible, and old behaviour can be easily obtained by removing any prior.

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)

Copy link
Collaborator

@frgsimpson frgsimpson left a comment

Choose a reason for hiding this comment

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

Looks good!

trieste/models/gpflow/utils.py Show resolved Hide resolved
@hstojic
Copy link
Collaborator

hstojic commented Nov 24, 2023

@uri-granta did you check @j-wilson PR where he also addressed the same issue: #794?

@uri-granta
Copy link
Collaborator Author

@uri-granta did you check @j-wilson PR where he also addressed the same issue: #794?

AFAIU that's doing more than just fixing the bug in randomize_hyperparameters. Will look at that as soon as this is merged.

@uri-granta uri-granta merged commit 45fbc37 into develop Nov 27, 2023
12 checks passed
@uri-granta uri-granta deleted the uri/793/randomize_hyperparameters_prior_on branch November 27, 2023 11:59
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