Skip to content

[tune] support hierarchical search spaces for hyperopt#11431

Merged
richardliaw merged 3 commits intoray-project:masterfrom
krfricke:tune-hyperopt-hierarchical
Oct 19, 2020
Merged

[tune] support hierarchical search spaces for hyperopt#11431
richardliaw merged 3 commits intoray-project:masterfrom
krfricke:tune-hyperopt-hierarchical

Conversation

@krfricke
Copy link
Copy Markdown
Contributor

@krfricke krfricke commented Oct 16, 2020

Why are these changes needed?

Adds support for hierarchical search spaces using hyperopt.

This will now work with HyperOpt:

config = {
    "a": 1,
    "dict_nested": tune.choice([{
        "a": tune.choice(["M", "N"]),
        "b": tune.choice(["O", "P"])
    }]),
    "list_nested": tune.choice([
        [
            tune.choice(["M", "N"]),
            tune.choice(["O", "P"])
        ],
        [
            tune.choice(["Q", "R"]),
            tune.choice(["S", "T"])
        ],
    ]),
    "domain_nested": tune.choice([
        tune.choice(["M", "N"]),
        tune.choice(["O", "P"])
    ]),
}

and result in configs like this:

{'a': 1, 'dict_nested': {'a': 'M', 'b': 'P'}, 'list_nested': {0: 'N', 1: 'O'}, 'domain_nested': 'P'}
{'a': 1, 'dict_nested': {'a': 'N', 'b': 'O'}, 'list_nested': {0: 'R', 1: 'T'}, 'domain_nested': 'M'}
{'a': 1, 'dict_nested': {'a': 'N', 'b': 'P'}, 'list_nested': {0: 'Q', 1: 'T'}, 'domain_nested': 'M'}
{'a': 1, 'dict_nested': {'a': 'M', 'b': 'O'}, 'list_nested': {0: 'Q', 1: 'T'}, 'domain_nested': 'O'}
{'a': 1, 'dict_nested': {'a': 'N', 'b': 'P'}, 'list_nested': {0: 'M', 1: 'O'}, 'domain_nested': 'N'}
{'a': 1, 'dict_nested': {'a': 'N', 'b': 'P'}, 'list_nested': {0: 'R', 1: 'T'}, 'domain_nested': 'M'}
{'a': 1, 'dict_nested': {'a': 'M', 'b': 'P'}, 'list_nested': {0: 'R', 1: 'S'}, 'domain_nested': 'O'}

Related issue number

Closes #11216

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@krfricke krfricke self-assigned this Oct 16, 2020
@krfricke krfricke mentioned this pull request Oct 16, 2020
Closed
@richardliaw
Copy link
Copy Markdown
Contributor

richardliaw commented Oct 16, 2020

@amogkam can you review? ping me when approved + tests passing - thanks!

@richardliaw richardliaw removed their request for review October 16, 2020 17:19
amogkam
amogkam approved these changes Oct 16, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@amogkam amogkam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just left one clarification comment mostly for my understanding. Looks like the test is timing out though.

python/ray/tune/suggest/hyperopt.py Outdated
category, prefix=par)
if isinstance(category, dict) else
HyperOptSearch.convert_search_space(
dict(enumerate(category)), prefix=f"par/{i}")
Copy link
Copy Markdown
Contributor

@amogkam amogkam Oct 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is prefix used for/why is the variable par used as the prefix for dicts but as a string for lists? How is this reflected in the config dict?

Copy link
Copy Markdown
Contributor Author

@krfricke krfricke Oct 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an error here, thanks - I'll fix it. Basically we need the prefix to avoid duplicate labels in hyperopt

Copy link
Copy Markdown
Contributor Author

@krfricke krfricke Oct 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So par is always a string. Dicts then have their subkeys, so it will be a/x, a/y anyway. But lists have to be converted to dicts in order to be convertable, and we then end up with a/0, a/1, etc. Resolving the values should be fine, as we end up with dicts with integer indices - they are not lists, but can still be accessed with numerical indices. See also the test for this.

Copy link
Copy Markdown
Contributor

@amogkam amogkam Oct 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, makes sense

@amogkam
Copy link
Copy Markdown
Contributor

amogkam commented Oct 19, 2020

@richardliaw tune tests are passing. should be ready to merge.

@richardliaw richardliaw merged commit ed81010 into ray-project:master Oct 19, 2020
@krfricke krfricke deleted the tune-hyperopt-hierarchical branch September 22, 2023 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amogkam amogkam amogkam approved these changes

Assignees

@amogkam amogkam

@krfricke krfricke

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Tune] Allow for Hierarchical Search Spaces in Ray1.0

3 participants

@krfricke @richardliaw @amogkam