Add Mlflow integration callback. #1028

PhilipMay · 2020-03-14T23:00:53Z

This is the PR for a Mlflow integration callback. Also see the discussion here: #1017

Todo

type annotations
extend setup.py
tests
docstring

PhilipMay · 2020-03-16T19:41:50Z

@sile @toshihikoyanase @g-votte
Can someone of you please help me fix this "error" from the ci:

#!/bin/bash -eo pipefail
. venv/bin/activate
mypy .

optuna/integration/mlflow.py:49: error: "FixedTrial" has no attribute "number"
optuna/integration/mlflow.py:51: error: "FixedTrial" has no attribute "value"
optuna/integration/mlflow.py:60: error: "FixedTrial" has no attribute "number"
optuna/integration/mlflow.py:62: error: "FixedTrial" has no attribute "datetime_complete"
optuna/integration/mlflow.py:63: error: "FixedTrial" has no attribute "state"
Found 5 errors in 1 file (checked 122 source files)

Exited with code exit status 1

g-votte · 2020-03-17T06:03:56Z

@PhilipMay
Thanks for your PR! Let us take some time for reviewing.

Can someone of you please help me fix this "error" from the ci:

I think it's because the expected type is not FixedTrial but FrozenTrial.

PhilipMay · 2020-03-17T06:38:20Z

@g-votte thanks. Now I have this issue:

#!/bin/bash -eo pipefail
. venv/bin/activate
autopep8 . -r --diff --exit-code

/bin/bash: line 1: autopep8: command not found

Exited with code exit status 127

g-votte · 2020-03-17T07:33:08Z

@PhilipMay
The following PR might be the cause, where autopep8 has just been replaced with black.
Could you try merging and resolving the latest master?

#1030

codecov-io · 2020-03-18T18:38:50Z

Codecov Report

Merging #1028 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1028   +/-   ##
=======================================
  Coverage   90.40%   90.40%           
=======================================
  Files         128      128           
  Lines       11159    11159           
=======================================
  Hits        10088    10088           
  Misses       1071     1071

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f90719d...f90719d. Read the comment docs.

g-votte

Although some more changes would be done later for setup.py and test cases, I put several comments for the current implementation.

optuna/integration/mlflow.py

tests/integration_tests/test_mlflow.py

optuna/integration/mlflow.py

PhilipMay · 2020-03-23T10:18:09Z

@g-votte @toshihikoyanase
All tests are green. I am done with this PR. Can you do a final review please?

g-votte

Thanks a lot for you continuously working on this feature! Although I still would like add more detailed review later, I put a few comments.

optuna/integration/mlflow.py

setup.py

tests/integration_tests/test_mlflow.py

optuna/integration/mlflow.py

PhilipMay · 2020-03-24T20:35:58Z

Thanks for your comments and recommendations. I incorporated everything. Are there any more issues you see?

tests/integration_tests/test_mlflow.py

PhilipMay · 2020-03-26T06:44:01Z

Merged and rebased master again.

g-votte

Thanks a lot for your quick and continuous actions! I put some additional comments.

I'd still like to work on some more review.

optuna/integration/mlflow.py

tests/integration_tests/test_mlflow.py

PhilipMay · 2020-03-27T21:09:44Z

Resolved all review items and merged + rebased master again.

optuna/integration/mlflow.py

hvy

Thanks for the PR and sorry for my late participation. I left some comments if you could take a look.

optuna/integration/mlflow.py

Co-Authored-By: Hiroyuki Vincent Yamazaki <hiroyuki.vincent.yamazaki@gmail.com>

Co-Authored-By: Shotaro Sano <g-votte@users.noreply.github.com>

PhilipMay · 2020-04-15T05:00:08Z

Merged and rebased master again.

harupy · 2020-04-15T05:16:37Z

@PhilipMay

Thanks for the comment!

Lets try to bring this PR to a good 80 % solution and we can add more features later when someone wants to take time to code them.

You're right. I agree😄

PhilipMay · 2020-04-15T05:59:44Z

All remarks are handled, code changes accepted and tests are green.

g-votte

LGTM. We may want to follow up the nested run question and other nits after merging this PR.

@PhilipMay
Thanks a lot for your long-running contribution!

@harupy
Thank you very much for your feedback!

PhilipMay · 2020-04-15T16:40:51Z

@PhilipMay
Thanks a lot for your long-running contribution!

Yes - thanks. Was fun with you implementing and discussing this stuff. Although I initialy thought that it would be easier and quicker I learned a lot from your comments and remarks.

PhilipMay mentioned this pull request Mar 14, 2020

MLflow integration #1017

Closed

toshihikoyanase added the feature Change that does not break compatibility, but affects the public interfaces. label Mar 16, 2020

PhilipMay changed the title ~~Mlflow integration callback~~ Add Mlflow integration callback. Mar 16, 2020

PhilipMay force-pushed the mlflow branch 3 times, most recently from 3fd9e52 to c348e8e Compare March 18, 2020 18:14

g-votte suggested changes Mar 21, 2020

View reviewed changes

PhilipMay force-pushed the mlflow branch from d907aca to a4e9137 Compare March 21, 2020 06:51

PhilipMay requested a review from g-votte March 21, 2020 07:07

g-votte self-assigned this Mar 21, 2020

PhilipMay force-pushed the mlflow branch from 1020069 to d0b0a7c Compare March 23, 2020 07:03

g-votte suggested changes Mar 24, 2020

View reviewed changes

optuna/integration/mlflow.py Outdated Show resolved Hide resolved

setup.py Outdated Show resolved Hide resolved

tests/integration_tests/test_mlflow.py Outdated Show resolved Hide resolved

harupy reviewed Mar 24, 2020

View reviewed changes

optuna/integration/mlflow.py Outdated Show resolved Hide resolved

g-votte suggested changes Mar 26, 2020

View reviewed changes

tests/integration_tests/test_mlflow.py Outdated Show resolved Hide resolved

PhilipMay force-pushed the mlflow branch from 603503c to 52cbf20 Compare March 26, 2020 06:42

g-votte suggested changes Mar 27, 2020

View reviewed changes

optuna/integration/mlflow.py Outdated Show resolved Hide resolved

tests/integration_tests/test_mlflow.py Outdated Show resolved Hide resolved

tests/integration_tests/test_mlflow.py Outdated Show resolved Hide resolved

tests/integration_tests/test_mlflow.py Outdated Show resolved Hide resolved

PhilipMay force-pushed the mlflow branch 2 times, most recently from e3b77b0 to 74843df Compare March 27, 2020 21:08

PhilipMay requested a review from g-votte March 31, 2020 07:18

g-votte suggested changes Apr 1, 2020

View reviewed changes

hvy requested changes Apr 1, 2020

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 Show resolved Hide resolved

optuna/integration/mlflow.py Outdated Show resolved Hide resolved

PhilipMay and others added 20 commits April 15, 2020 06:58

import order fix

702faaf

fixed "Incompatible types in assignment" warning

7fafb9b

changed experimental vers. to 1.4.0

5aedfff

MLflow experiment name must be set with study_name

57e7c0b

formatting fix

504aeb1

formatting fix

8211720

just using study_name for MLflow experiment

e8e5d69

removed unused import

6afab0f

Better metric_name handling

517b877

Improved docstring

fa37949

removed optional from param

f2e7753

Co-Authored-By: Hiroyuki Vincent Yamazaki <hiroyuki.vincent.yamazaki@gmail.com>

simplified docstring

b345e9c

Co-Authored-By: Hiroyuki Vincent Yamazaki <hiroyuki.vincent.yamazaki@gmail.com>

removed unused test func param

938b4d7

Writing Optuna with big O in docstrings

28f2d9b

removed unneeded assert

8e0df56

trimming study direction

cecd027

added assert in test again to enforce float type

9f65696

improved docstring for metric_name

fdce692

hardcoded x_max

fcf68fe

Co-Authored-By: Shotaro Sano <g-votte@users.noreply.github.com>

dry tests

3449947

Co-Authored-By: Shotaro Sano <g-votte@users.noreply.github.com>

PhilipMay force-pushed the mlflow branch from f99c23f to 3449947 Compare April 15, 2020 04:58

fixed move of FrozenTrial

f90719d

PhilipMay requested a review from g-votte April 15, 2020 05:59

g-votte approved these changes Apr 15, 2020

View reviewed changes

g-votte merged commit d3b4f50 into optuna:master Apr 15, 2020

toshihikoyanase mentioned this pull request May 22, 2020

Omniboard-ish frontend #728

Closed

Uh oh!

Add Mlflow integration callback. #1028

Add Mlflow integration callback. #1028

Uh oh!

Conversation

PhilipMay commented Mar 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo

Uh oh!

PhilipMay commented Mar 16, 2020

Uh oh!

g-votte commented Mar 17, 2020

Uh oh!

PhilipMay commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

g-votte commented Mar 17, 2020

Uh oh!

codecov-io commented Mar 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

g-votte left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhilipMay commented Mar 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

g-votte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhilipMay commented Mar 24, 2020

Uh oh!

Uh oh!

PhilipMay commented Mar 26, 2020

Uh oh!

g-votte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhilipMay commented Mar 27, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hvy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PhilipMay commented Apr 15, 2020

Uh oh!

harupy commented Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PhilipMay commented Apr 15, 2020

Uh oh!

g-votte left a comment

Choose a reason for hiding this comment

Uh oh!

PhilipMay commented Mar 14, 2020 •

edited

Loading

PhilipMay commented Mar 17, 2020 •

edited

Loading

codecov-io commented Mar 18, 2020 •

edited

Loading

g-votte left a comment •

edited

Loading

PhilipMay commented Mar 23, 2020 •

edited

Loading

harupy commented Apr 15, 2020 •

edited

Loading