Skip to content

Refactor save_to_hub, and add option to push training checkpoints to Hub#1629

Closed
NimaBoscarino wants to merge 8 commits intohuggingface:masterfrom
NimaBoscarino:master
Closed

Refactor save_to_hub, and add option to push training checkpoints to Hub#1629
NimaBoscarino wants to merge 8 commits intohuggingface:masterfrom
NimaBoscarino:master

Conversation

@NimaBoscarino
Copy link
Copy Markdown
Contributor

This PR contributes two things:

  1. The save_to_hub method has been refactored and simplified to use one of the new utility functions from the Hugging Face Hub client library, which uses the create_commit API.
  2. A feature has been added to allow .fit() to push checkpoints to the Hub.

Pushing the checkpoints halts training (and there are no progress bars for the upload), which might not be ideal UX. Would it be better if the upload happened in the background while training continued?

@NimaBoscarino
Copy link
Copy Markdown
Contributor Author

cc: @omarespejel @osanseviero

Copy link
Copy Markdown
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Very nice! Thanks for the PR 🔥 Do you have an example repo created with this?

@NimaBoscarino
Copy link
Copy Markdown
Contributor Author

@osanseviero Yup! Here's an example repo: https://huggingface.co/NimaBoscarino/STPushToHub-test

@NimaBoscarino
Copy link
Copy Markdown
Contributor Author

@osanseviero I refactored it and now have a HubParameters class to hold the params! Here's a repo where I tested it all out: https://huggingface.co/NimaBoscarino/July25Test/commits/main

If you think this looks good, I can write up some docstrings for it and I can add it to the Hugging Face section of the docs.

Copy link
Copy Markdown
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Thanks a lot! This is looking quite good, I left some final comments

Copy link
Copy Markdown
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

This looks good to me!

  • Feel free to close discussions a resolved if they are resolved :)
  • I left a comment about local_repo_path creating temporary directories

APart from that, this looks in good state. Pinging @nreimers 🤗 to see what they think

@tomaarsen
Copy link
Copy Markdown
Member

Hello!

This PR has been superseded by #2380. Also, Repository is now outdated. I do want to thank you for this time, and I'm sorry that it was never merged - I recognize that you (all) put time and effort into this.

  • Tom Aarsen

@tomaarsen tomaarsen closed this Dec 18, 2023
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.

3 participants