search

secondmind-labs / trieste

A Bayesian optimization toolbox built on TensorFlow
Apache License 2.0
212 stars 41 forks source link

Refactor `SingleObjectiveTrustRegion*` classes to make them more generic #835

Closed khurram-ghani closed 2 months ago

khurram-ghani commented 3 months ago

Related issue(s)/PRs: None

Summary

This PR refactors SingleObjectiveTrustRegionBox and SingleObjectiveTrustRegionDiscrete classes to reduce code duplication, plus it introduces more flexibility for algorithms (e.g. concept of location_candidate). There is a new common class called HypercubeTrustRegion.

Fully backwards compatible: yes

PR checklist

khurram-ghani commented 2 months ago

@hstojic thanks for the review. Did you have a name suggestion for GenericUpdatableTrustRegion? I think it could have "Box" somewhere in the name because that is how it defines local neighbours, for both continuous and discrete variables. It then grows and shrinks these boxes. However, we can't use "Box" because Trieste uses that to mean continuous spaces, and that would be confusing.

hstojic commented 2 months ago

Not a strong opinion at the end as I don't have a clear enough view on how classes are used and the differences between them

Perhaps if its specific in a way that its using hyper rectangles rather than other types such as ellipsoid's, we could put that in the name?Hypercube actually?

-------- Original Message -------- On 23/04/2024 13:11, Khurram Ghani wrote:

@.***(https://github.com/hstojic) thanks for the review. Did you have a name suggestion for GenericUpdatableTrustRegion. I think it could have "Box" somewhere in the name because that is how it defines local neighbours, for both continuous and discrete variables. It then grows and shrinks these boxes. However, we can't use "Box" because Trieste uses that to mean continuous spaces, and that would be confusing.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

khurram-ghani commented 2 months ago

@hstojic on the hierarchy of classes, I had discussions with @uri-granta about that and he is happy with it :-)