Skip to content
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 CategoricalHyperparameters #166

Open
Yard1 opened this issue Oct 1, 2020 · 4 comments · May be fixed by #346
Open

Proposal to handle None in CategoricalHyperparameters #166

Yard1 opened this issue Oct 1, 2020 · 4 comments · May be fixed by #346

Comments

@Yard1
Copy link
Contributor

Yard1 commented Oct 1, 2020

As explained in #104, None cannot be used as a possible value because CS uses it to mark disabled hyperparameters. This can cause issues when None 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 by isinstance(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

@simonprovost
Copy link

@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 CategoricalHyperparameters with an boolean additional parameter. This parameter could indicate whether the hyperparameter is optional in the search space.

This approach would allow users to indicate optional hyperparameters in a more explicit and intuitive way. Instead of using the value None or the string "None", which could lead to confusion or errors, users could simply set this new parameter to indicate that a hyperparameter might not be used while getting selected/called.

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 None with dummy classes.

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 "None".

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!

@eddiebergman
Copy link
Contributor

eddiebergman commented Jun 16, 2023

Thanks for the input @simonprovost, I think the main issue with your proposal is simply that None does not mean that it's unused, sometimes we have something like "red", "blue", None, which are all valid options which SMAC should search over. A proto-typical example is where "red" means use the "red" option, "blue" means use the blue option and None could be a flag that indicates some internal hueristic optimization.

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 None.

In principal, I think @Yard1's suggestion could work, except rather than use a class, we can use a Byte enum or something, typically these are called sentinal values, which are very useful when the typical null-type of a language is a valid value and is distinct from the absence of a value.

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 None could backfire:

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 None should be a valid option, I would hope they realise this user error. But this behavior could definitely be already present in tools that use ConfigSpace.

@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 "None" trick, however newer users could use this and not realise that their optimizer has not accounted for this.

@simonprovost
Copy link

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!

@eddiebergman
Copy link
Contributor

Hiyo, this has been handled in #346 and the follow up #359 which handles arbitrary objects in a categorical, of which None is of course one of them. Hopefully released next week!

@eddiebergman eddiebergman linked a pull request Apr 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants