secondmind-labs / trieste

A Bayesian optimization toolbox built on TensorFlow
Apache License 2.0
219 stars 42 forks source link

Multi trust regions #773

Closed khurram-ghani closed 1 year ago

khurram-ghani commented 1 year ago

Related issue(s)/PRs: None Related branch: victor/trustregions

Summary

This PR adds support for multi-trust-regions acquisition rule; based-on and refactored from branch victor/trustregions.

The main classes are:

The intention is that whenever a new trust region algorithm is implemented, in most cases, it would be sufficient to simply sub-class UpdatableTrustRegion and BatchTrustRegion.

Question: since SingleObjectiveTrustRegionBox/BatchTrustRegionBox is a specific trust-regions implementation, should it be moved to either a separate file or perhaps to a notebook? Is it generally useful? Answer: yes these are useful and should be kept with the rules for now.

An example notebook will be added in a follow-on PR.

Fully backwards compatible: yes

PR checklist

khurram-ghani commented 1 year ago

trying to make up my mind about updatable class - are there efficiency gains with that?

@hstojic There aren't computation efficiency gains. This is about good design patterns (encapsulation); what is easy for developers to understand and maintain. In my opinion, having a separate update-able class makes it clear what it does and keeps most of the code self contained. Only when something applies to multiple regions (e.g. maybe_reinitialise_subspaces), it exists in the rule iteself.