-
Notifications
You must be signed in to change notification settings - Fork 89
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
Proposal to handle None
in CategoricalHyperparameter
s
#166
Comments
@eddiebergman, has there, by any luck, been any progress on this issue? @mfeurer may have been very busy at that time when @Yard1 mentioned him. In the meantime, and in relation to the SMAC workflow, plus acknowledging that I might not be fully aware of all the contexts in which ConfigSpace is used, I would like to suggest a different approach than what @Yard1 proposed: Rather than introducing a dummy class to indicate unused hyperparameters, which could potentially add unnecessary memory overhead, we could instead enhance This approach would allow users to indicate optional hyperparameters in a more explicit and intuitive way. Instead of using the value In practice, if this parameter is set for a given hyperparameter and this hyperparameter is not "chosen" as if it were "None", the training function (refer to SMAC) would not even have to manage the case because this hyperparameter would not even be passed. This increases the user's control and clarity over the configuration of their search space while introducing less complication when "None" is selected. It may be less prone to errors or misunderstandings than the current usage of '"None"' or the proposed In the long run, this approach could make the handling of optional hyperparameters more user-friendly and less reliant on hacky workarounds. However, it would involve changes to the API, so it would need to be introduced carefully, possibly alongside a deprecation process for the current usage of I'm interested in hearing your thoughts on this proposal! I could not provide anything right now so I might use "None" - but definitely could provide something over the summer if interested or if another design you reckon might be more efficient? Cheers! |
Thanks for the input @simonprovost, I think the main issue with your proposal is simply that def find_optimal_color(c: Literal["red", "blue"] | None = "red") -> ...:
if c is None:
c = "purple"
... In the above example, optionally disabling a hyperparameter would not have the desired effect as we explicitly want to pass In principal, I think @Yard1's suggestion could work, except rather than use a I could look into it but the primary issue is really the knock on effect downstream with respect to SMAC or any other optimizer. I'm kind of okay with this breaking change though as it should be done at some point because I've run into this issue quite a few times. As an example where allowing space = ConfigurationSpace({
"c": ["red", "green", None],
"do_color_opt": [True, False],
})
space.add_conditional(
InCondition(space["c"], space["do_color_opt"], [True])
)
config = space.sample_configuration()
# User error happens here
color = config.get("c")
if color is not None:
find_optimal_color(color) The correct user behaviour would be sentinel = object()
color = config.get("c", sentinel)
if color is not sentinel:
find_optimal_color(color) However, given the user explicitly encoded that @mfeurer Do you know if this would have a downstream effect on something like SMAC? I don't think it breaks backwards compatibility since anyone who has encountered this issue has likely done the |
It makes perfect sense! Yes, I believe this could be considered an enhancement, as this query is certainly not posed by the first user. Furthermore, the Byte enum is a great alternative to the dummy class indeed. Thank you for the quick knowledge bonus, greatly appreciated! |
As explained in #104,
None
cannot be used as a possible value because CS uses it to mark disabled hyperparameters. This can cause issues whenNone
is a possible value for a hyperparameter, necessitating workarounds like using a string. However, I would like to propose a solution that should be simple to implement.Instead of using
None
to mark disabled hyperparameters, a dummy class would be created for that sole purpose, with null checks replaced byisinstance(x, dummy_class)
checks. That class would be only used internally.I believe that to be a quick and viable solution to this issue. I would be happy to work on it myself, but I do not know the codebase well enough to do it without assistance. @mfeurer
The text was updated successfully, but these errors were encountered: