-
Notifications
You must be signed in to change notification settings - Fork 352
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
Simplify configurable optimizer API #518
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would something like this be more natural to use?
budget: Optional[int] = None, | ||
num_workers: int = 1, | ||
scale: float = 1.0, | ||
diagonal: bool = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify extra configuration parameters with defaults here (as you usually do)
@@ -239,8 +247,8 @@ def _internal_provide_recommendation(self) -> ArrayLike: | |||
return self.es.result.xbest # type: ignore | |||
|
|||
|
|||
class ParametrizedCMA(base.ParametrizedFamily): | |||
"""TODO | |||
class ParametrizedCMA(base.ConfiguredOptimizer): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then just create a Wrapper class
MilliCMA = ParametrizedCMA(scale=1e-3).with_name("MilliCMA", register=True) | ||
MicroCMA = ParametrizedCMA(scale=1e-6).with_name("MicroCMA", register=True) | ||
CMA = ParametrizedCMA().set_name("CMA", register=True) | ||
DiagonalCMA = ParametrizedCMA(diagonal=True).set_name("DiagonalCMA", register=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And decline the different values of the extra parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also updated an example you are familiar with: SplitOptimizer3,5,9,13 :D
) -> None: | ||
super().__init__(parametrization, budget=budget, num_workers=num_workers) | ||
if num_vars: | ||
if num_optims: | ||
if num_vars is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@teytaud make if num_vars
can lead to unexpected behavior if what you wanted is to test agains None
, so always use if num_vars is not None
num_vars: Optional[List[Any]] = None | ||
) -> None: | ||
super().__init__(parametrization, budget=budget, num_workers=num_workers, num_optims=num_optims, num_vars=num_vars) | ||
SplitOptimizer3 = ConfSplitOptimizer(num_optims=3).set_name("SplitOptimizer3", register=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all it takes now to have all the variants
Agreed by @teytaud , merging and following with other PRs to propagage |
Types of changes
Motivation and Context / Related issue
The simplified process means:
How Has This Been Tested (if it applies)
Checklist