Update Keras Mixin #284
Update Keras Mixin #284
Conversation
There was a problem hiding this comment.
This is great, thanks for working on this @nateraw! Really stoked about this refactor :)
| def save_pretrained_keras( | ||
| model, save_directory: str, config: Optional[Dict[str, Any]] = None | ||
| ): | ||
| """Saves a Keras model to save_directory in SavedModel format. Use this if you're using the Functional or Sequential APIs. |
There was a problem hiding this comment.
I think the decision to switch from .h5 to SavedModel is sound.
I think we should monitor issues closely to ensure that this doesn't create an incompatibility/incomprehension layer between transformers which outputs .h5 files and huggingface_hub which outputs SavedModel format.
src/huggingface_hub/keras_mixin.py
Outdated
| if use_auth_token is None and repo_url is None: | ||
| token = HfFolder.get_token() | ||
| if token is None: | ||
| raise ValueError( | ||
| "You must login to the Hugging Face hub on this computer by typing `transformers-cli login` and " | ||
| "entering your credentials to use `use_auth_token=True`. Alternatively, you can pass your own " | ||
| "token as the `use_auth_token` argument." | ||
| ) | ||
| elif isinstance(use_auth_token, str): | ||
| token = use_auth_token | ||
| else: | ||
| token = None | ||
|
|
||
| if repo_path_or_name is None: | ||
| repo_path_or_name = repo_url.split("/")[-1] | ||
|
|
||
| # If no URL is passed and there's no path to a directory containing files, create a repo | ||
| if repo_url is None and not os.path.exists(repo_path_or_name): | ||
| repo_name = Path(repo_path_or_name).name | ||
| repo_url = HfApi(endpoint=api_endpoint).create_repo( | ||
| token, | ||
| repo_name, | ||
| organization=organization, | ||
| private=private, | ||
| repo_type=None, | ||
| exist_ok=True, | ||
| ) |
There was a problem hiding this comment.
This logic should already be handled by the __init__ of Repository.
The initialization can currently take care of:
- Setting the correct token according to
use_auth_tokenaccording tostr/bool - Identifying the repo name from the URL
- Creating the repository if it doesn't exist.
Could you try working with it? Please let me know if you run into any issues, hopefully it makes your life easier.
|
It would be nice to also add a Github Actions job with Keras to test this automatically on CI! |
Co-authored-by: Lysandre Debut <[email protected]>
nateraw
force-pushed
the
keras-savedmodel
branch
from
123aa09 to
8182c32
Compare
| @@ -57,6 +57,23 @@ jobs: | |||
|
|
|||
| - run: pytest -sv ./tests/ | |||
|
|
|||
| build_tensorflow: | |||
|
I came late to the last round of review, but great work! 🔥 |
Both mentioned in #230
Note - I had to switch to using
snapshot_downloadbecause:keras_model.pbmetadata.pb,variables/, etc.Using
forced_filenamealso could have worked, but you'd have to gather up all the files withinvariables/assets/etcI've seen some folks just zip up the specific file tree instead due to similar issues.