-
Notifications
You must be signed in to change notification settings - Fork 443
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
[WIP] Add enhancement for Parameter Distribution #2059
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,128 @@ | ||||||||||||||||||||||||
# Proposal for Parameter Distribution | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
<!-- toc --> | ||||||||||||||||||||||||
- [Summary](#summary) | ||||||||||||||||||||||||
- [Motivation](#motivation) | ||||||||||||||||||||||||
- [Design Details](#design-details) | ||||||||||||||||||||||||
- [Experiment API changes](#experiment-api-changes) | ||||||||||||||||||||||||
- [Correspondence for Katib Distributions and Framework Distributions](#correspondence-for-katib-distributions-and-framework-distributions) | ||||||||||||||||||||||||
- [Chocolate](#chocolate) | ||||||||||||||||||||||||
- [Goptuna](#goptuna) | ||||||||||||||||||||||||
- [Hyperopt](#hyperopt) | ||||||||||||||||||||||||
- [Optuna](#optuna) | ||||||||||||||||||||||||
- [Scikit-Optimize](#scikit-optimize) | ||||||||||||||||||||||||
<!-- /toc --> | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
## Summary | ||||||||||||||||||||||||
This enhancement introduces `Distribution` to tuning parameters and remove redundantly `ParameterType`. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
API field in the Experiment spec determine parameter type with distribution. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
## Motivation | ||||||||||||||||||||||||
Currently, Katib does not support determining a distribution for search space that samplers pick up parameters by users. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
Katib should be able to determine it by users since | ||||||||||||||||||||||||
almost hyperparameter tuning algorithms (framework) can determine it by users. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
## Proposal | ||||||||||||||||||||||||
We introduce a mechanism to determine a distribution for search space by users. | ||||||||||||||||||||||||
That also means we introduce a mechanism to propagate distributions to suggestion-services and | ||||||||||||||||||||||||
set them samplers of each framework. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
## Design Details | ||||||||||||||||||||||||
The proposal consists of a new Experiment API field and | ||||||||||||||||||||||||
correspondence between Katib Distributions and Framework Distributions. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
### Experiment API changes | ||||||||||||||||||||||||
We extend the Experiment API to introduce the new fields `Distribution` to configure the distribution for search space and | ||||||||||||||||||||||||
remove the redundant fields `ParameterType`. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
```diff | ||||||||||||||||||||||||
type ParameterSpec struct { | ||||||||||||||||||||||||
Name string `json:"name,omitempty"` | ||||||||||||||||||||||||
- ParameterType ParameterType `json:"parameterType,omitempty"` | ||||||||||||||||||||||||
+ Distribution Distribution `json:"distribution,omitempty"` | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great proposal @tenzen-y . One question, how do we provide backward compatibility? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnugeorge This is a good point. For the time being (1~2 releases?), I think we can operate Also, we should add webhook validation to restrict @andreyvelich @johnugeorge wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM. Also, add deprecation tag to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
SGTM katib/pkg/apis/manager/v1beta1/api.proto Line 83 in f941ec6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we add only new features to v1beta2 API, deprecation labels are unnecessary since we create a separate proto definition for v1beta2 API as discussed in #2059 (comment). |
||||||||||||||||||||||||
FeasibleSpace FeasibleSpace `json:"feasibleSpace,omitempty"` | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
- type ParameterType string | ||||||||||||||||||||||||
+ type Distribution string | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const ( | ||||||||||||||||||||||||
- ParameterTypeUnknown ParameterType = "unknown" | ||||||||||||||||||||||||
- ParameterTypeDouble ParameterType = "double" | ||||||||||||||||||||||||
- ParameterTypeInt ParameterType = "int" | ||||||||||||||||||||||||
- ParameterTypeDiscrete ParameterType = "discrete" | ||||||||||||||||||||||||
- ParameterTypeCategorical ParameterType = "categorical" | ||||||||||||||||||||||||
+ UnknownDistribution Distribution = "unknown" | ||||||||||||||||||||||||
+ CategoricalDistribution Distribution = "categorical" | ||||||||||||||||||||||||
+ IntUniformDistribution Distribution = "intUniform" | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use camel case here? Personally prefer lower case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM |
||||||||||||||||||||||||
+ IntLogUniformDistribution Distribution = "intLogUniform" | ||||||||||||||||||||||||
+ FloatUniformDistribution Distribution = "floatUniform" | ||||||||||||||||||||||||
+ FloatLogUniformDistribution Distribution = "floatLogUniform" | ||||||||||||||||||||||||
Comment on lines
+59
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @johnugeorge @tenzen-y @gaocegege @anencore94 What do you think about following hyperopt model instead of Also, Ray Tune follows the same model: https://docs.ray.io/en/latest/tune/api_docs/search_space.html, and NNI has the same APIs: https://nni.readthedocs.io/en/stable/hpo/search_space.html#quniform There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@andreyvelich Sounds good. I would add the corresponding tables for the old ParameterType and new Distribution using the hyperopt model to this proposal.
@andreyvelich Sounds good. One question, Does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM. While I am thinking if this is a huge change to our YAML APIs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe, we need to change the API version to v1beta2. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM
I will create a correspondence table between v1beta1 and v1beta2. Maybe, we only need to create a table for the
IIUC, we do not need to install manifests for conversion webhook to clusters. ref:
IMO, we need to keep maintaining v1beta1 for at least one release version. This means if we introduce v1beta2 API in katib v0.16.0, we will remove v1beta1 API in katib v0.17.0. @gaocegege Do you know how many release versions we kept maintaining v1alpha2 after we introduced v1beta1? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@andreyvelich @gaocegege Maybe, custom (implemented by user) suggestion services using v1beta1 API will not work since gRPC calls are not through conversion webhook. <------------------------------ [Updated] ------------------------------
So, I would like to work in the following:
------------------------------ [Updated] ------------------------------>
katib/pkg/util/v1beta1/katibconfig/config.go Lines 35 to 45 in db72ce1
<------------------------------ [Updated] ------------------------------
------------------------------ [Updated] ------------------------------>
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, gRPC might be a problem, yes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tenzen-y Also, are we going to rename There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@andreyvelich Kubernetes uses helper functions to convert multiple APIs. Does that answer your question?
Yes, I updated the above comment. |
||||||||||||||||||||||||
) | ||||||||||||||||||||||||
``` | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
### Correspondence between Katib Distributions and Framework Distributions | ||||||||||||||||||||||||
We extend suggestion services to be able to configure distributions for | ||||||||||||||||||||||||
search space using libraries provided in each framework. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
#### Chocolate | ||||||||||||||||||||||||
TODO | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blocked by #2058 |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
#### Goptuna | ||||||||||||||||||||||||
We can extend Goptuna Suggestion Service using Goptuna libraries shown in the below correspondence table for | ||||||||||||||||||||||||
Katib Distributions and Goptuna Distributions. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
ref: https://github.com/c-bata/goptuna/blob/2245ddd9e8d1edba750839893c8a618f852bc1cf/distribution.go | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| Katib Distribution | Goptuna Distribution | | ||||||||||||||||||||||||
|-----------------------------|------------------------------------------------------| | ||||||||||||||||||||||||
| CategoricalDistribution | CategoricalDistribution | | ||||||||||||||||||||||||
| IntUniformDistribution | IntUniformDistribution or StepIntUniformDistribution | | ||||||||||||||||||||||||
| IntLogUniformDistribution | IntUniformDistribution or StepIntUniformDistribution | | ||||||||||||||||||||||||
| FloatUniformDistribution | UniformDistribution | | ||||||||||||||||||||||||
| FloatLogUniformDistribution | LogUniformDistribution | | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
#### Hyperopt | ||||||||||||||||||||||||
We can extend Hyperopt Suggestion Service using Hyperopt libraries shown in the below correspondence table for | ||||||||||||||||||||||||
Katib Distributions and Hyperopt Distributions. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
ref: http://hyperopt.github.io/hyperopt/getting-started/search_spaces/#parameter-expressions | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| Katib Distribution | Hyperopt Distribution | | ||||||||||||||||||||||||
|-----------------------------|-----------------------| | ||||||||||||||||||||||||
| CategoricalDistribution | hp.choice | | ||||||||||||||||||||||||
| IntUniformDistribution | hp.quniform | | ||||||||||||||||||||||||
| IntLogUniformDistribution | hp.qloguniform | | ||||||||||||||||||||||||
| FloatUniformDistribution | hp.quniform | | ||||||||||||||||||||||||
| FloatLogUniformDistribution | hp.qloguniform | | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
#### Optuna | ||||||||||||||||||||||||
We can extend Optuna Suggestion Service using Optuna libraries shown in the below correspondence table for | ||||||||||||||||||||||||
Katib Distributions and Optuna Distributions. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
ref: https://optuna.readthedocs.io/en/stable/reference/distributions.html | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| Katib Distribution | Optuna Distribution | | ||||||||||||||||||||||||
|-----------------------------|---------------------------------------| | ||||||||||||||||||||||||
| CategoricalDistribution | distributions.CategoricalDistribution | | ||||||||||||||||||||||||
| IntUniformDistribution | distributions.IntDistribution | | ||||||||||||||||||||||||
| IntLogUniformDistribution | distributions.IntDistribution | | ||||||||||||||||||||||||
| FloatUniformDistribution | distributions.FloatDistribution | | ||||||||||||||||||||||||
| FloatLogUniformDistribution | distributions.FloatDistribution | | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
#### Scikit Optimize | ||||||||||||||||||||||||
We can extend Scikit-Optimize Suggestion Service using Scikit-Optimize libraries shown in the below correspondence table for | ||||||||||||||||||||||||
Katib Distributions and Scikit-Optimize Distributions. | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
ref: https://scikit-optimize.github.io/stable/modules/classes.html#module-skopt.space.space | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| Katib Distribution | Scikit-Optimize Distribution | | ||||||||||||||||||||||||
|-----------------------------|------------------------------| | ||||||||||||||||||||||||
| CategoricalDistribution | space.Categorical | | ||||||||||||||||||||||||
| IntUniformDistribution | space.Integer | | ||||||||||||||||||||||||
| IntLogUniformDistribution | space.Integer | | ||||||||||||||||||||||||
Comment on lines
+125
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess, you can set uniform or loguniform in skopt using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's right. We need to set the prior argument in skopt. Also, we need to set the log argument in optuna. |
||||||||||||||||||||||||
| FloatUniformDistribution | space.Real | | ||||||||||||||||||||||||
| FloatLogUniformDistribution | space.Real | |
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.
Please can you link the appropriate issue: #1207 to this proposal motivation?
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.
Makes sense.