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.

Sign up for GitHub

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

Jump to bottom

Make requests in chunks due to limitations imposed by MLflow #3530

Closed
wants to merge 4 commits into from
Closed

Make requests in chunks due to limitations imposed by MLflow #3530

wants to merge 4 commits into from

Conversation

grafail
Copy link
Contributor

@grafail grafail commented May 3, 2022

Motivation

Push fails in MLflow callback in case there are more than 100 parameters and tags or 1000 metrics, due to restrictions posed by MLflow (https://www.mlflow.org/docs/latest/rest-api.html#request-limits).

Description of the changes

  • Split information sent to MLflow into chunks of 100 or 1000

@github-actions github-actions bot added the optuna.integration Related to the `optuna.integration` submodule. This is automatically labeled by github-actions. label May 3, 2022
@grafail grafail mentioned this pull request May 3, 2022
Closed
@nzw0301 nzw0301 self-assigned this May 4, 2022
nzw0301
nzw0301 requested changes May 4, 2022
View reviewed changes
Copy link
Member

@nzw0301 nzw0301 left a comment

Choose a reason for hiding this comment

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

Thank you for sending this PR to improve mlflow callback. Could you take a look at my comments?
In addition, could you add tests to https://github.com/optuna/optuna/blob/master/tests/integration_tests/test_mlflow.py ?

optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
nzw0301
nzw0301 requested changes May 6, 2022
View reviewed changes
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
nzw0301
nzw0301 requested changes May 6, 2022
View reviewed changes
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
optuna/integration/mlflow.py Outdated Show resolved Hide resolved
@HideakiImamura
Copy link
Member

@xadrianzetx Could you review this PR if you have time? Please feel free to remove the assignment if you are busy.

@xadrianzetx
Copy link
Collaborator

Sure!

@nzw0301
Copy link
Member

nzw0301 commented May 15, 2022

hi @grafail, could you need our help to update this PR?

@grafail
Copy link
Contributor Author

grafail commented May 15, 2022

Hello, sorry for the delay, have been quite busy throughout the last week. I will update it by tomorrow.

@nzw0301
Copy link
Member

nzw0301 commented May 15, 2022

Thank you for your reply! I don't intend to force you to work quickly. Please take the time since you were busy last week!

@grafail grafail force-pushed the fix/mlflow-callback-chunking branch from 2ddd4ac to fa2733d Compare May 16, 2022 08:49
nzw0301
nzw0301 requested changes May 16, 2022
View reviewed changes
Copy link
Member

@nzw0301 nzw0301 left a comment

Choose a reason for hiding this comment

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

Thanks! We need to fix CI for docs.

optuna/integration/mlflow.py Outdated Show resolved Hide resolved
@contramundum53 contramundum53 added the enhancement Change that does not break compatibility and not affect public interfaces, but improves performance. label May 23, 2022
nzw0301
nzw0301 reviewed May 26, 2022
View reviewed changes
optuna/integration/mlflow.py
from typing import List
from typing import Optional
from typing import Sequence
from typing import Union


Copy link
Member

Choose a reason for hiding this comment

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

Let's fix isort error.

Suggested change

@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2022

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label Jun 5, 2022
@nzw0301 nzw0301 mentioned this pull request Jun 9, 2022
Merged
2 tasks
@nzw0301
Copy link
Member

nzw0301 commented Jun 9, 2022

I'll takeover this PR due to its inactive state for while.

@nzw0301 nzw0301 closed this Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nzw0301 nzw0301 nzw0301 requested changes

Assignees

@nzw0301 nzw0301

@xadrianzetx xadrianzetx

Labels
enhancement Change that does not break compatibility and not affect public interfaces, but improves performance. optuna.integration Related to the `optuna.integration` submodule. This is automatically labeled by github-actions. stale Exempt from stale bot labeling.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@grafail @HideakiImamura @xadrianzetx @nzw0301 @contramundum53