-
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
[SDK] Add 'algorithm_settings' in client tune #2227
[SDK] Add 'algorithm_settings' in client tune #2227
Conversation
@@ -141,6 +141,7 @@ def tune( | |||
base_image: str = constants.BASE_IMAGE_TENSORFLOW, | |||
namespace: Optional[str] = None, | |||
algorithm_name: str = "random", | |||
algorithm_settings: Union[dict, list[models.V1beta1AlgorithmSetting], None] = 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.
I am wondering should we discuss idea that @droctothorpe suggested here: #2225 (comment).
E.g. we can create separate Class for every Suggestion that Katib supports and add algorithm settings, search space, other features for this suggestion: katib.suggestion.optuna
, katib.suggestion.skopt
.
Different Suggestions might have different supported features that we can add to these classes.
For example: @DeukWoongWoo asked for callable search space support for Optuna: #2228
I understand that will add maintain overhead, but user will get better experience.
We need to discuss what is the easiest way for the user to get access for Katib Suggestion features.
What are your thoughts @johnugeorge @tenzen-y @droctothorpe @shipengcheng1230 @DeukWoongWoo @a9p @c-bata @anencore94 ?
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.
We are building similar configuration objects internally for stuff like dask and pytorch. Code completion, type hints, and robust docstrings make managing complex configurations a lot easier. All for it.
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.
Love the idea for a better user experience! How do you think using pydantic for this checking/validation capability?
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.
@andreyvelich
I totally agree completely with the statement: "We need to discuss what is the easiest way for the user to get access to Katib Suggestion features." However, I think it's important to note that this PR is primarily aimed at enabling the setting of algorithm settings in katib_client, which is not currently possible as mentioned in the "search-algorithms-in-detail" guide.
It's a bit unclear whether what you're trying to say is discussing features that will be added, or if you're focused on discussing how users will be able to access input into algorithm settings as this PR.
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.
Hey @andreyvelich, as @DeukWoongWoo suggested, do you feel comfortable to let this PR just enable the specification and have future one to validate and give suggestions to the input of algorithm settings?
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.
Thanks everyone for your comments!
We are building similar configuration objects internally for stuff like dask and pytorch.
@droctothorpe Would you mind share some details here ? Is there something that we can adopt with Katib too ?
It's a bit unclear whether what you're trying to say is discussing features that will be added, or if you're focused on discussing how users will be able to access input into algorithm settings as this PR.
I just want to discuss how to streamline experience for users to use Suggestions that Katib supports (e.g. Optuna, Hyperopt, etc.). Currently, it's not clear which underlying framework we use for HP tuning algorithms until user goes to Search Algorithm in Detail guide.
Also, there are overlaps in supported algorithms between different frameworks: https://www.kubeflow.org/docs/components/katib/katib-config/#suggestion-settings.
E.g. tpe
is supported by Hyperopt and Optuna (I assume that 2 frameworks might have different features that we can use in Katib + Kubernetes).
Thus, users need to understand relationship between Katib Experiment + Katib Config to understand the flow.
As an user experience example, in Ray tune they wrap every supported framework in a separate module (e.g. ray.tune.search.optuna
) that might have custom settings.
Wondering what would be the optimal experience for Katib users.
do you feel comfortable to let this PR just enable the specification and have future one to validate and give suggestions to the input of algorithm settings?
Yeah, sure, I don't want to block this PR, just to hear your thoughts.
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.
@droctothorpe Would you mind share some details here ? Is there something that we can adopt with Katib too ?
Happy to elaborate. Imagine something like this:
from typing import Literal, Optional
class BayesianOptimizationSettings:
def __init__(
base_estimator: Literal["GP", "RF", "ET", "GBRT"] = "GP",
n_initial_points: int = 10,
acq_func: str = "gp_hedge",
acq_optimizer: Literal["sampling", "lbfgs", "auto"] = "auto",
random_state: Optional[int] = None
):
self.base_estimator = base_estimator
# etc.
This would include comprehensive docstrings, code completion, and input validation, so you wouldn't have to reference an external resource outside of the SDK. It could optionally leverage ipywidgets to do something like this:
You could then instantiate the config object, edit properties on the fly (not just at instantiation time) with the benefit of code completion, type hints, and docstrings, then pass the configuration object to the tune
method.
Just a thought. FWIW, it's a bigger lift (and a much bigger operational burden) than this PR and should probably not be a condition for merging it.
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.
Thanks for sharing, actually, that is similar to what I proposed: #2227 (comment), but using Suggestion instead of Algorithm.
Do we want to create separate issue to track this improvement @droctothorpe @shipengcheng1230 ?
2798d30
to
d741d6f
Compare
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.
Thanks for the update @shipengcheng1230!
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andreyvelich, shipengcheng1230 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@kubeflow/wg-automl-leads Could you restart failed CI jobs? It seems that some jobs failed due to issues regarding reachability to the container registry. |
What this PR does / why we need it:
The
algorithm_settings
cannot be set in thetune
method of the python SDK, therefore many details here cannot be immediately used if not submitting the manifest directly.This PR adds the capability to do it. By default,
algorithm_settings
is set toNone
which aligns with the initiation ofV1beta1AlgorithmSpec
. So when not settingalgorithm_settings
the behavior is not changed.Which issue(s) this PR fixes:
Fixes #2225