Added MLflow callback

[cacharle]

Here is a small draft for #769, I feel like the different loggers have a lot in common and could be refactored in a common class.

[BenjaminBossan]

Thank you for working on this. I have a few change suggestions how to possibly change this. Please tell me what you think.

As you probably saw, this implementation is a bit different than those for tensorboard, neptune, sacred, etc. They all use an object that encompasses something like an "experiment" or "session", which is passed to the logger and is used to run all the methods on. Here, instead we have instead the local import of mlflow. If possible, I would like to see the same design pattern used here as for the other loggers.

Without any knowledge of mlflow, I took a dive through the code and it seems like under the hood, mlflow.log_metric is indeed just a shortcut to retrieve an MlflowClient, on which the log_metric method is called. So I wonder if we can make use of that, similar to how it's shown in the mlflow docs here. Since you know mlflow much better, do you believe this is feasible?

And before doing any in depth review and testing out mlflow, could you please rename the callback to MlflowLogger or something similar, and also add tests? Again, take inspiration from (and copy) code from the other loggers.

[BenjaminBossan]

@cacharle Any updates, do you need support?

[cacharle]

Hi, found a way to use a Run and MlflowClient instance instead of calling the the basic mlflow API.
I'll try to add the tests by this weekend.

[BenjaminBossan]

Please tell me once the PR is ready for review.

[BenjaminBossan]

Thanks a lot for addressing my initial comments. I tested the MLFlow logger out and it worked well. So this is a very welcome addition.

There are a few things that I would like you to address before merging, please take a look at the comments. On top of that, could you please add an entry to the CHANGES.md? Thanks.

[cacharle]

Should I squash all the commit I made before merging the PR to avoid cluttering the logs?

[BenjaminBossan]

Should I squash all the commit I made before merging the PR to avoid cluttering the logs?

Please don't, it makes reviewing harder. I will squash before merging :)

[BenjaminBossan]

Thanks for addressing the comments. The MLFlow logger is almost good to go, there are only a few minor comments left and a minor merge conflict to solve.

[BenjaminBossan]

Thanks for implementing these changes and going through the extra effort of linking to the MLFlow docs. Now there is only the merge conflict left to solve :)

[BenjaminBossan]

Thanks, great work!