remove redundant PieceWiseLinearGP add ExactW Kernels#750
Merged
Conversation
jduerholt
requested changes
May 29, 2026
Contributor
jduerholt
left a comment
There was a problem hiding this comment.
Looks really, really good to me. I love it. Only some minor comments.
Collaborator
Author
|
How about this @jduerholt |
jduerholt
approved these changes
Jun 1, 2026
Contributor
jduerholt
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
With the advent of the engineered features, we can now, in principle, get rid of the
PieceWiseLinearGPbecause it can be fully recreated using theInterpolateFeatureengineered feature. So, for future maintenance, it is better to remove the now-redundant PieceWiseLinearGP and accompanying code from, for instance,torch_tools.pyand other locations.In the meantime, I also worked on exact implementations of the
WassersteinKernel. TheWassersteinKernelwas computed on an interpolated grid. TheExactWassersteinKernelconsiders the union of unique x-values and then uses the trapezoidal rule, together with identifying the sign of break points, to perform the computation exactly. Both W1 and W2 distances are supported. The W1 implementation matches the approximateWassersteinKernelat an adequately high number of interpolation points.The union of unique x-values can become quite large when having high-dimensional problems, a large number of raw samples/restarts in the acq. func. optimization, etc. Which on moderate compute (e.g., small k8 compute), can lead to memory issues. Therefore, there is also some chunking in the code so that we loop over the union. For CPU this was also slighly faster. Thoughts on this implementation @jduerholt?
Lastly, fixed a bug in the interp1d, which could lead to gradfNone errors due to division by zero. In principle, this should never happen, as inequality constraints are set, but initial candidates provided by Botorch sometimes violate these.
Have you read the Contributing Guidelines on pull requests?
Yes
Have you updated
CHANGELOG.md?Yes
Test Plan
Removed old tests.
Created new tests for new features.
Ran tests.