NumericToCategoricalEncoding Input Transform. #2907
Conversation
facebook-github-bot
added
the
CLA Signed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2907 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 212 216 +4
Lines 19778 20285 +507
==========================================
+ Hits 19778 20285 +507 ☔ 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.
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.
botorch/models/transforms/input.py
Outdated
| """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.
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.
There was a problem hiding this comment.
I added an example ;)
| 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.
this arg name could be more descriptive, e.g. numeric_cardinality or sth like that
There was a problem hiding this comment.
Hmm, I was naming it like this as it also called like this in the related OneHotToNumeric transform. What do you think about categorical_features_cardinality?
There was a problem hiding this comment.
hmm yeah we should keep things consistent with OneHotToNumeric. I don't think the arg name is ideal (cc @sdaulton) but I don't know if it warrants changing things in OneHotToNumeric and having to deal with backward compatibility issues. So this seems ok to me.
|
Thanks for the review, I will go over it and adapt accordingly. Best, Johannes |
|
Hi @Balandat, I have now implemented all your suggestions besides the one regarding the name, what do you think about my proposal there? Best, Johannes |
| 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.
hmm yeah we should keep things consistent with OneHotToNumeric. I don't think the arg name is ideal (cc @sdaulton) but I don't know if it warrants changing things in OneHotToNumeric and having to deal with backward compatibility issues. So this seems ok to me.
Co-authored-by: Max Balandat <Balandat@users.noreply.github.com>
|
@Balandat: Should be done now ;) |
3 participants
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.