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

Add: mlflow for experiment tracking #1059

Merged
merged 11 commits into from
Jan 9, 2024

Conversation

JohanWork
Copy link
Contributor

Adding MLFOW to Axolotl for experiment tracking, looked into how Weight and Bias has been setup and tried to follow the same pattern. Have tested the changes and everything looks good to me.

Happy for any feedback or comments.

@JohanWork JohanWork changed the title Adding mlflow for experiment tracking Add: mlflow for experiment tracking Jan 7, 2024
Copy link
Collaborator

@winglian winglian left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@@ -694,6 +694,10 @@ wandb_name: # Set the name of your wandb run
wandb_run_id: # Set the ID of your wandb run
wandb_log_model: # "checkpoint" to log model to wandb Artifacts every `save_steps` or "end" to log only at the end of training

# mlflow configuration if you're using it
# Make sure your `MLFLOW_TRACKING_URI` is set.
mlflow_experiment_name: # Your experiment name
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth logging a warning when this is set but the uri env isnt.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be better to make a config mlflow_tracking_uri: ? The parsing code should be able to set that if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with what you propose @NanoCode012 . Have update the code.

Update mlflow_tracking_uri
@NanoCode012
Copy link
Collaborator

Is there any need to pass this config to the HF Trainer to enable log? I think some validation test would also be needed to make sure both aren't active at same time.

In addition, would you be able to provide an example run output + image on mlflow?

@winglian
Copy link
Collaborator

winglian commented Jan 8, 2024

I disagree, isn't the tracking uri equivalent to a secret token? It can get inadvertently exposed if you share your YAML (say integration with wandb also)

@JohanWork
Copy link
Contributor Author

I disagree, isn't the tracking uri equivalent to a secret token? It can get inadvertently exposed if you share your YAML (say integration with wandb also)

To my understanding it is only an url actually. In Mlflow auth seams to be handled with username and password. But I might miss something. For reference https://mlflow.org/docs/latest/auth/index.html

I have no strong opinion if it should be or not be in the config file.

@NanoCode012
Copy link
Collaborator

Oh, I wasn't aware it was a secret. In this case, I would not recommend it being in the yaml anymore due to reasons wing mentioned.

update trainer building
@winglian
Copy link
Collaborator

winglian commented Jan 8, 2024

looks like the URI isn't considered a secret, but could be sensitive if someone starts a server and doesn't enable auth on it.
MLFLOW_TRACKING_USERNAME and MLFLOW_TRACKING_PASSWORD are the respective credentials

@winglian winglian merged commit 090c24d into axolotl-ai-cloud:main Jan 9, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants