[Tune] Nevergrad optimizer with extra parameters

[yhna940]

Why are these changes needed?

Some Nevergrad search algorithms have required inputs, such as budget for the NgOpt search algorithm, but it is not possible with the NevergradSearch class to pass these parameters down to the search algorithm. I would propose adding something like optimizer_kwargs to the NevergradSearch that get passed to the optimizer when instantiating it.

Related issue number

#26305

Checks

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• 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 :(

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

• If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[yhna940]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

• If you'd like to keep this open, just leave any comment, and the stale label will be removed.

.

[krfricke]

cc @justinvyu can you help shepherd this?

[justinvyu]

Thanks for this contribution! I have a few minor suggestions, and a note about the failing test:

We should fix this test. Currently, it's passing in a parameter_names argument that's not being used at all and is being filled in as the optimizer_kwargs, causing the error. We should just remove this.

ray/python/ray/tune/tests/test_tune_restore_warm_start.py

Line 253 in a205fda

class NevergradWarmStartTest(AbstractWarmStartTest, unittest.TestCase):

Also, while we're at it, can we not accept **kwargs in the NevergradSearch constructor? It's currently not used at all and would create some confusion between the added optimizer_kwargs and the regular kwargs.

[justinvyu]

Can we limit the test to just the searcher initialization? Just making sure that initializing NevergradSearch with an optimizer that requires args without erroring should be good enough.

[yhna940]

I totally agree with you. Therefore, for this test case, only verify that nevergrad is instantiated. thanks.

[yhna940]

We should fix this test. Currently, it's passing in a parameter_names argument that's not being used at all and is being filled in as the optimizer_kwargs, causing the error. We should just remove this.

ray/python/ray/tune/tests/test_tune_restore_warm_start.py

Line 253 in a205fda

class NevergradWarmStartTest(AbstractWarmStartTest, unittest.TestCase):

Fixed by specifying parameter_names. I am so sorry.

[yhna940]

Also, while we're at it, can we not accept **kwargs in the NevergradSearch constructor? It's currently not used at all and would create some confusion between the added optimizer_kwargs and the regular kwargs.

**kwargs is only used to call __init__ of NevergradSearch's parent class.
The parameters used to build the optimizer used in NevergradSearch need to be defined separately.
As you said, optimizer_kwargs is too confusing, so we have replaced it with optimizer_configs.
Thank you so much for your review.

[justinvyu]

Thanks!

In my comment early, I meant removing this **kwargs entirely and not passing it into the super().__init__(). The base Searcher class doesn't actually take in any args other than metric and mode, so it's not needed. Sorry for the confusion!

Could you also merge with the latest master? (Make sure not to rebase! It will introduce a bunch of commits to this PR.)

[yhna940]

Thanks!

In my comment early, I meant removing this **kwargs entirely and not passing it into the super().__init__(). The base Searcher class doesn't actually take in any args other than metric and mode, so it's not needed. Sorry for the confusion!

Could you also merge with the latest master? (Make sure not to rebase! It will introduce a bunch of commits to this PR.)

Thank you for your review. There were 3 major changes.

1. Renamed back to optimizer_kwargs.

2. Merged with latest branch changes.

3. Removed kwargs in super __init__ method.

[justinvyu]

Thank you for the contribution! I am retrying a few of the failed tests - they shouldn't be affected by these changes. Should be good to merge after they pass!

[krfricke]

Thanks!