search

secondmind-labs / trieste

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

TURBO #739

Closed henrymoss closed 1 year ago

henrymoss commented 1 year ago

Hi Y'all I've implemented a different trust region strategy (TURBO).

pls review

henrymoss commented 1 year ago

@uri-granta ive done a lot more fiddling. Please can you fix the types for me.

uri-granta commented 1 year ago

@uri-granta ive done a lot more fiddling. Please can you fix the types for me.

What's the deal with get_local_dataset (added in one commit, removed in another, but still mentioned)?

henrymoss commented 1 year ago

@uri-granta ive done a lot more fiddling. Please can you fix the types for me.

What's the deal with get_local_dataset (added in one commit, removed in another, but still mentioned)?

@uri-granta sorry that was my mistake. should be back now

uri-granta commented 1 year ago

@uri-granta ive done a lot more fiddling. Please can you fix the types for me.

What's the deal with get_local_dataset (added in one commit, removed in another, but still mentioned)?

@uri-granta sorry that was my mistake. should be back now

Some more issues:

rule.py:1181 How are you going to initialize failure_tolerance if the rule is not a DiscreteThompsonSampling? rule.py:1256 You don't need to redeclare self._local_models's type here (mypy should be able to figre out it's not None). test_rule.py:844 Should assert that _local_models is NOT None after acquire. test_rule.py:844+ Should assert the precise type of the model: at the moment all mypy knows is that it's a TrainableSupportsGetKernel, which doesn't necessarily have a .kernel (though it does have get_kernel()) or ._dataset.

uri-granta commented 1 year ago

@henrymoss Are you happy to merge this or is there anything else you still want to change? The format, typing and docs issues are all fixed.