-
Notifications
You must be signed in to change notification settings - Fork 42
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
Notebook for trust regions #777
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its ok, but I would get back the visualisation of the TR boxes, see my comment below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if it would make sense to have a first section showing TREGO. Or ideally, TREGO, then batch-TR with Thompson sampling, and finally TurBO?
So that this notebook would give an overview of the TR features we have in trieste and would also act as an integration test?
97b954c
to
d0b85d4
Compare
Yes, makes sense. I'll add that in the notebook using the existing versions of TREGO and TurBO. We can then migrate to the batch implementations later in the respective PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, few minor comments
# | ||
# Finally, we show how to run Bayesian optimization with the `TurBO` algorithm. This is a | ||
# trust region algorithm that uses local models and datasets to approximate the objective function | ||
# within one trust region. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what the right wording is, but perhaps "within their respective trust regions"? otherwise it could sound like a single TR algorithm? ignore if you think this is sufficiently clear as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did have slightly different wording before that implied multiple regions, but explicitly changed it to one region. The current TurBO implementation only supports one region. When that is replaced with new classes in a future PR, I can update the wording.
Related issue(s)/PRs: #773
Summary
Added example notebook for three trust region algorithms.
Fully backwards compatible: yes
PR checklist