-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat/update mlflow-related sample scripts #12471
base: master
Are you sure you want to change the base?
feat/update mlflow-related sample scripts #12471
Conversation
…s return version set
…at/update-mlflow-ui
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (81.81%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.
... and 34 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
# Create model group | ||
model_group_urn = client.create_model_group( | ||
group_id="airline_forecast_models_group", | ||
properties=models.MLModelGroupPropertiesClass( |
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.
better abstraction! also how about putting detail of properties class into create_model_group method ? then customer doesn't have to know properties class. (but I'd like to know pros/cons of doing it )
), | ||
) | ||
|
||
run_urn = client.create_training_run( |
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 method make abstraction better!
- nit on naming, create_run is shorter? (unless run is overloaded term) or experiment_run ?
- how about allowing to put experiment_urn in argument, then create run-> experiment relationship automaticaly? (then user do not call add_run_to_experiment)
) | ||
|
||
# Creating a model with property classes | ||
model_urn = client.create_model( |
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.
how about adding model_group_run as argument, then user do not have to call add_model_to_model_group
# Creating a model with property classes | ||
model_urn = client.create_model( | ||
model_id="arima_model", | ||
properties=models.MLModelPropertiesClass( |
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.
IMO, this also expose internal details of properties class to user, but since this properties class is complicated, hiding it might have pros/cons. might be helpful to add one more helper function for metrics/param? (not strong opinion)
Checklist