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.

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

mlflow run context is not logged when using MLFlowLogger #6745

Closed
stllfe opened this issue Mar 30, 2021 · 0 comments · Fixed by #6746
Closed

mlflow run context is not logged when using MLFlowLogger #6745

stllfe opened this issue Mar 30, 2021 · 0 comments · Fixed by #6746
Labels
bug Something isn't working help wanted Open to be worked on

Comments

@stllfe
Copy link
Contributor

stllfe commented Mar 30, 2021

🐛 Bug

When we use the basic mlflow logging via with mlflow.start_run(): ... context manager, we get a better supplementary info about the run (git commit sha, user, filename) rendered in the Tracking UI (system tags)

But when we use MLFlowLogger as a logger in pytorch_lightning, this info is not logged. As a user, I'd like to have a mirrored functionality out-of-the-box.

I inspected the start_run() method of mlflow and deduced that the only thing is left while creating the run via MLflowClient is to add resolve_tags from the context package:

# pytorch_lightning/loggers/mlflow.py
from mlflow.tracking.context.registry import resolve_tags
...
    def experiment(self) -> MLflowClient:
        if self._run_id is None:
            run = self._mlflow_client.create_run(experiment_id=self._experiment_id, tags=resolve_tags(self.tags))

I think it's a better idea to add those tags internally (meaning not to expect users doing that manually) as first - it's as seamless as in the default API, secondly - it's the pytorch_lightning that manages the mlflow's run anyways.

PR is following ...

@stllfe stllfe added bug Something isn't working help wanted Open to be worked on labels Mar 30, 2021
Borda added a commit that referenced this issue Apr 8, 2021
* Update mlflow.py

#6745 adds additional info about the run, as in the native API

* Update mlflow.py

trying to fix some backward compatibility issues with `resolve_tags`

* wip on backward compatibility

added a default for `getattr` in case the `registry` object exists, but has no proper attribute (weird case but who knows...)

* fix pep

* impoert

* fix registry import

* try fix failing tests

removed the first if statement, so that `resolve_tags` would be defined either case

* fix formatting

Co-authored-by: Jirka Borovec <jirka.borovec@seznam.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Open to be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant