Support for Tune search spaces

[Yard1]

Adds support to Tune's search spaces, resolving https://github.com/ray-project/tune-sklearn/issues/124

[Yard1]

WIP. Will work more on it. No support for grid search yet (though do we need it?). @krfricke FYI

CI will fail as the functionality (https://github.com/ray-project/ray/pull/11503) is not yet in ray stable, or whatever version the CI is pulling.

[Yard1]

Also @richardliaw looks like CI is doing pip install -U https://s3-us-west-2.amazonaws.com/ray-wheels/latest/ray-0.9.0.dev0-cp36-cp36m-manylinux1_x86_64.whl. Shouldn't it be switched to ray-1.0.0?

[richardliaw]

hmm, can we update it to 1.1.0.dev0? (see the wheel links here https://docs.ray.io/en/latest/installation.html)

[Yard1]

@krfricke How are tune search spaces handled in grid search? The only thing left for this PR is determining how to handle grid searching. I think that it would be fine to leave it as-is (that is only support a dict of lists instead of any search spaces), but I am open to suggestions.

[richardliaw]

@Yard1 feel free to ping Kai again, perhaps on slack, after 48 hours :)

[krfricke]

In Tune, grid search is represented by a Categorical domain with a Grid sampler. You can also use the Domain.is_grid() method to determine if a domain should be grid searched.

In Tune, grid search means that all combinations of all grid search items (if there is more than one grid search item) should be generated at least once. E.g. with "a": tune.grid_search([1, 2]) the basic variant generator will generate two trials, one with a=1 and one with a=2. The num_samples parameter will multiply this number, i.e. if num_samples=2, four trials will be generated here. Does this help?

[Yard1]

@krfricke So how would a user pass search spaces to be grid searched?

[krfricke]

So in regular Ray Tune a user would pass something like

config = {
    "var": tune.grid_search([1, 2, 3]),
    "rnd": tune.uniform(0, 5)
}
tune.run(
    trainable,
    config=config)

With num_samples=1 this would create three trials, one for each grid serach value (1, 2 and 3). The rnd value is drawn randomly for each of these trials.

[Yard1]

Great, thanks. This clears it up.

[Yard1]

@richardliaw @krfricke Tests are passing, Mac fails due to an outdated Ray version, I think. I'll double check, but in the meantime, it's ready for review.

It's also not working with the currently released Ray 1.0.0 as it's missing the functionality @krfricke added later. Scratch that!

[richardliaw]

1.0.1 is out now :)

[Yard1]

@richardliaw Great to hear that! I need to put on notifications for the main repo, hah

[richardliaw]

This looks good! though:

• it seems to be failing on mac build?
• can we update the readme/docstrings?

Also, @krfricke please take a look!

[Yard1]

@richardliaw Updated docstrings and readme. Not sure what's up with Mac, I'll look into it tomorrow. Maybe it's downloading the wrong version?

[Yard1]

@krfricke Mac CI fails on test_timeout, could you take a look?

[krfricke]

Yep, I'll look into it tomorrow

[krfricke]

Seems there's some more overhead for the test_timeout (18.47 seconds instead of 18). Without the time budget the test would run for 50+ seconds. Should we just raise that limit to 25 to account for environment/MacOS overhead? Do you just want to include it in this PR since it is a 1 line change?

[Yard1]

@krfricke Yeah, sure. Do you want to make the change or should I?

[richardliaw]

Antoni, probably faster for yourself to change it ;)

On Thu, Nov 12, 2020 at 12:48 PM Antoni Baum notifications@github.com wrote:

@krfricke https://github.com/krfricke Yeah, sure. Do you want to make the change or should I?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ray-project/tune-sklearn/pull/128#issuecomment-726333563, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCRZZKRYUCAN6O2NYFF2XTSPRCYJANCNFSM4TGXZLQA .

[Yard1]

@richardliaw Touché!

[Yard1]

@richardliaw Mac now errors instead of failing. It was doing that with other PRs - I think it's something to do with running out of memory? I think it should be good to merge though (once Kai has a chance to review), considering Linux passes.

[richardliaw]

hmmm it looks like Mac tests are failing but it's not an out of memory issue?

[Yard1]

@richardliaw Looks like the functionality needed for this PR is not yet in release Ray - that's what Mac uses, and it's what the exception on it is related to.

[richardliaw]

Got it - a couple things

1. We can introduce new features (without regressions) not compatible with released Ray, but we must make sure the user knows what is the error.
2. We should avoid introducing a failing test into CI; it should be marked as skipped if anything.

Thanks!

[Yard1]

@richardliaw Got it. I think we could just wait a while on this PR, unless we are planning to release a new version of tune-sklearn soon.