Skip to content

Support MLFlowLogger with only the mlflow-skinny dependency installed #16486

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

Closed
dconathan opened this issue Jan 24, 2023 · 4 comments · Fixed by #16513
Closed

Support MLFlowLogger with only the mlflow-skinny dependency installed #16486

dconathan opened this issue Jan 24, 2023 · 4 comments · Fixed by #16513
Assignees
Labels
feature Is an improvement or enhancement logger: mlflow

Comments

@dconathan
Copy link
Contributor

dconathan commented Jan 24, 2023

Bug description

Sorry not sure to label this as a "feature" or a "bug":

mlflow-skinny is a variant of the mlflow library that installs the bare-minimum required for tracking: https://pypi.org/project/mlflow-skinny/

As far as I can tell, this meets the requirements for using the MLFlowLogger, but currently it doesn't work because:

https://github.com/Lightning-AI/lightning/blob/4a802e00a843264afd5aea023d8475d4f1b4d360/src/pytorch_lightning/loggers/mlflow.py#L40

checks that precisely the mlflow library is installed. If this line were replaced with:

_MLFLOW_AVAILABLE = RequirementCache("mlflow>=1.0.0") or RequirementCache("mlflow-skinny>=1.0.0")

It seems to work fine.

Should I make a PR for this? Would we want to add separate tests for mlflow and mlflow-skinny environments?

How to reproduce the bug

No response

Error messages and logs

No response

Environment

No response

More info

No response

cc @Borda

@dconathan dconathan added the needs triage Waiting to be triaged by maintainers label Jan 24, 2023
@carmocca
Copy link
Contributor

Sounds good to me. You will probably need separate variables for each check in case some features are not supported with the skinny version

@carmocca carmocca added feature Is an improvement or enhancement logger: mlflow needs triage Waiting to be triaged by maintainers and removed needs triage Waiting to be triaged by maintainers labels Jan 25, 2023
@carmocca carmocca removed the needs triage Waiting to be triaged by maintainers label Jan 25, 2023
carmocca added a commit that referenced this issue Jan 27, 2023
@adeandrade
Copy link

Why was this merged on 2.0 if it was broken on 1.9?

@carmocca
Copy link
Contributor

@adeandrade We only include bug-fixes in the 1.9.x releases. And this wasn't considered one because it's adding support for this alternative MLFlow package. Does our Logger not work with mlflow installed?

@adeandrade
Copy link

adeandrade commented Feb 12, 2023

@carmocca pytorch-lightning was working with mlflow-skinny on 1.8 and it broke on 1.9 but I guess mlflow-skinny was not considered before. It's OK. I'll stick to 1.8 until 2.0 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is an improvement or enhancement logger: mlflow
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants