emukit
emukit copied to clipboard
Add gradient calculation for the covariance between points in GPyModelWrapper
This change adds a method get_covariance_between_points_gradients
to the GPyModelWrapper
to facilitate obtaining the gradients of the corresponding get_covariance_between_points
method implemented in the wrapper.
These gradients are useful when optimising any batch acquisition function that relies on the covariance between batch points (such as Multi-point Expected Improvement) sequentially, i.e. when optimising with respect to one element of the batch only.
A second change introduced is vectorization of the gradient calculation for full covariance in the GPyModelWrapper
class. Replacing the for-loop with numpy array operations results in ~50% speed-up for larger batches of points (> 50).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Codecov Report
Merging #347 (6738faa) into master (b793989) will increase coverage by
0.02%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #347 +/- ##
==========================================
+ Coverage 89.43% 89.46% +0.02%
==========================================
Files 123 123
Lines 4051 4061 +10
Branches 462 461 -1
==========================================
+ Hits 3623 3633 +10
Misses 332 332
Partials 96 96
Impacted Files | Coverage Δ | |
---|---|---|
emukit/model_wrappers/gpy_model_wrappers.py | 84.66% <100.00%> (+1.09%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update b793989...6738faa. Read the comment docs.
Apologies for taking time on this one, it's a bit hard for me to make complete sense of this change. I promise to get through it!
No worries, let me know if there is anything I can do to explain and document it better in the code.
To answer the two points:
- This PR introduces a new method that isn't declared in any model interfaces. That's backwards from Emukit's point of view. Are you planning to use this method anywhere? Perhaps create a new acquisition? If so, let's define an interface, or augment an existing one if that makes more sense.
This is indeed intended for an implementation of a new acquisition. It comes in handy whenever optimizing any of many batch acquisition functions sequentially – i.e. by selecting optimising batch acquisition with respect to one point in the batch at a time. Currently, get_covariance_between_points
is part of the IEntropySearchModel
interface, so maybe something like IDifferentiableEntropySeachModel
would make sense?
- Both changes carry some pretty heavy matrix algebra, which is really hard to follow. Can you please add some comments throughout, to clarify what's going on? Thanks!
Fair point, I'll try and make it easier to follow.
Codecov Report
Merging #347 (c280f6d) into main (cdcb0d0) will decrease coverage by
0.02%
. The diff coverage is93.10%
.
@@ Coverage Diff @@
## main #347 +/- ##
==========================================
- Coverage 89.01% 88.99% -0.03%
==========================================
Files 128 128
Lines 4244 4254 +10
Branches 609 609
==========================================
+ Hits 3778 3786 +8
- Misses 369 371 +2
Partials 97 97
Impacted Files | Coverage Δ | |
---|---|---|
emukit/core/interfaces/__init__.py | 100.00% <ø> (ø) |
|
emukit/core/interfaces/models.py | 61.76% <60.00%> (-0.31%) |
:arrow_down: |
emukit/model_wrappers/gpy_model_wrappers.py | 84.24% <100.00%> (+0.55%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update cdcb0d0...c280f6d. Read the comment docs.
What about the interface discussion? Is this resolved @apaleyes ? Where exactly is the new method required in Emukit, i.e., is there an acquisition function that requires the method?
Hey @mmahsereci , good to hear from you again!
@BrunoKM promised us an acquisition later down the line. But that's a good point that I forgot all about - the function shall be there as an implementation of some interface. Thanks for catching!
That sounds great, thanks for catching me up @apaleyes! And, yes I am back now it seems. :)
@apaleyes sorry, resolving the interfaces has been on the stack of my todos for the past couple of weeks, I will try to get around to it soon.
Yes, this is for a new acquisition that is still in the pipeline :)
@BrunoKM are you still planning to work on this PR (and the follow up of it)? Or shall we just close it?
Thanks for the follow-up @apaleyes!
I'm still happy to finish off this PR (if I don't do it before the end-of-year, feel free to close it).
I've been a bit on the fence about whether to push the new acquisition soon afterwards. It's one with particularly tricky gradients: i.e. manually writing a gradient function for it in pure NumPy turned out to be a pain. The current implementation internally uses either jax/pytorch to automatically get the gradient function. I'd assume adding these dependencies is not in line with the philosophy of the rest of the EmuKit repo.
I'm still hoping I'll be able to add the manual gradients later down the line and push the new acquisition function, but due to limited time, I can't say how soon that might be.
I added an interface that specifies the covariance between points derivative method. Let me know your thoughts!
As for the new acquisition function, I think I'll be able to add it without an evaluate_with_gradients
method relatively soon. An update with evaluate_with_gradients
could follow later down the line.