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

[Feature] Add Trust-region algorithm searcher #33

Merged
merged 25 commits into from
Jun 14, 2022
Merged

Conversation

yhna940
Copy link
Contributor

@yhna940 yhna940 commented May 26, 2022

No description provided.

@yhna940 yhna940 force-pushed the feature/trust-region branch 4 times, most recently from b9a3acd to 2a38afe Compare May 30, 2022 01:48
@yhna940
Copy link
Contributor Author

yhna940 commented May 30, 2022

We chose this algorithm because it showed the best performance in bbo benchmark experiments.
It differs from the original implementation in that it supports max mode and discrete variables.

[Ref]
https://arxiv.org/pdf/2104.10201v2.pdf
https://arxiv.org/abs/1910.01739
https://github.com/uber-research/TuRBO

@github-actions
Copy link

github-actions bot commented May 30, 2022

Coverage report

The coverage rate is 85.17%

The branch rate is 66.75999999999999%

81% of new lines are covered.

Diff Coverage details (click to unfold)

mmtune/ray/searchers/init.py

100.0% of new lines are covered

mmtune/mm/tasks/disc_test_func.py

97.14285714285714% of new lines are covered

mmtune/mm/tasks/init.py

100.0% of new lines are covered

mmtune/mm/tasks/cont_test_func.py

94.24083769633508% of new lines are covered

mmtune/ray/searchers/trust_region.py

68.5344827586207% of new lines are covered

@yhna940 yhna940 marked this pull request as ready for review May 30, 2022 02:41
@yhna940 yhna940 requested review from nijkah and KKIEEK and removed request for nijkah May 30, 2022 02:53
@nijkah nijkah changed the title Feature/trust region [Feature] Add Trust-region algorithm searcher May 30, 2022
return _styblinksitang(x, 100)

def run(self, *args, **kwargs):
args = kwargs['args']
Copy link
Member

@nijkah nijkah May 30, 2022

Choose a reason for hiding this comment

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

How about using args = self.args as in other tasks?

return o

def run(self, *args, **kwargs):
args = kwargs['args']
Copy link
Member

@nijkah nijkah May 30, 2022

Choose a reason for hiding this comment

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

How about using args = self.args as in other tasks?

space: Optional[Union[Dict]] = None,
metric: Optional[str] = None,
mode: Optional[str] = None,
):
Copy link
Member

Choose a reason for hiding this comment

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

Docstring is required.

@nijkah
Copy link
Member

nijkah commented May 30, 2022

@yhna940
The coverage rate seems much low. More unit tests should be required.
Please let me know if you need any help filling it.

Comment on lines 32 to 65
def sphere(x: np.ndarray) -> float:
"""The most classical continuous optimization testbed.

If you do not solve that one then you have a bug.
"""
assert x.ndim == 1
return float(x.dot(x))
Copy link
Member

Choose a reason for hiding this comment

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

Should the previous class Sphere be deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the sphere function alone does not seem to be a sufficient optimization test, a new Test function including it has been added. I used the nevergrad benchmark (noise free version).

@yhna940 yhna940 marked this pull request as draft June 1, 2022 04:34
@yhna940 yhna940 force-pushed the feature/trust-region branch from 9e62f92 to 0e8e15c Compare June 10, 2022 06:19
@yhna940 yhna940 force-pushed the feature/trust-region branch from 0b9b559 to 9121ff2 Compare June 13, 2022 06:36
@yhna940 yhna940 requested a review from nijkah June 13, 2022 07:38
@yhna940 yhna940 marked this pull request as ready for review June 13, 2022 07:49
@yhna940 yhna940 merged commit 4d64042 into master Jun 14, 2022
@yhna940 yhna940 deleted the feature/trust-region branch June 14, 2022 06:08
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 this pull request may close these issues.

3 participants