-
Notifications
You must be signed in to change notification settings - Fork 431
NumericToCategoricalEncoding Input Transform. #2907
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
NumericToCategoricalEncoding Input Transform. #2907
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2907 +/- ##
===========================================
- Coverage 100.00% 99.99% -0.01%
===========================================
Files 212 216 +4
Lines 19778 20257 +479
===========================================
+ Hits 19778 20256 +478
- Misses 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@saitcakmak @Balandat any thoughts on this? It would be totally fine for me, if you say that you do not see this functionality directly in botorch. Then I would integrate it into our codebase, which is also totally fine ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation seems reasonable to me. I'd be curious to see a concrete example using this end-to-end, including fitting a model and optimizing the acquisition function.
Similar to @saitcakmak , I would also be curious about how this compares against using a kernel that may work on the categoricals directly.
"""Transform categorical parameters from an integer representation | ||
to a vector based representation like one-hot encoding or a descriptor | ||
encoding. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to have a description of how the columns in the output of the transform are organized. Ideally there would be a concrete example in the docstring.
|
||
Args: | ||
dim: The dimension of the numerically encoded input. | ||
categorical_features: A dictionary mapping the index of each |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this arg name could be more descriptive, e.g. numeric_cardinality
or sth like that
@@ -1625,6 +1625,122 @@ def _expanded_perturbations(self, X: Tensor) -> Tensor: | |||
return p.transpose(-3, -2) # p is batch_shape x n_p x n x d | |||
|
|||
|
|||
class NumericToCategoricalEncoding(InputTransform): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, should this not be CategoricalToNumeric
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I am not sure how to call this propely, my thought was that here the categorical variable is encoded in a numeric fashion (by an integer) and we transform it some kind of categorical encoding, but this is definitly not ideal. Should I just rename to CategoricalToNumeric
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But CategoricalToNumeric
is kind of strange in light of OneHotToNumeric
, as this transform is a generalized version of the inverese of OneHotToNumeric
, this is why I was naming it NumericToCategoricalEncoding
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm why is this strange?
Thanks for the review, I will go over it and adapt accordingly. Best, Johannes |
Motivation
This PR refers to #2879. It adds a new input transform that transforms a categorical degree of freedom encoded a an integer into some kind of vector based description. This could be for example a one-hot encoding, but also a descriptor encoding as it is often used in chemistry. It adds the possibility to use the alternating acqf optimizer also with surrogates that do not treat categoricals as integer based values. For example one could then also use a SAAS GP with the mixed alternating acqf optimizer and treat the categoricals under the hood as one-hots.
Have you read the Contributing Guidelines on pull requests?
Yes.
Test Plan
Unit tests, most of them are implemented (also to demonstrate the functionality), the ones which check the equality between transforms and correct behavior of transform on train etc. are still missing. My plan is to add them after a first feedback after a first review.