[cleanup] Offline mode and cache dir from huggingface_hub constants + cleanup in PushToHubMixin
#42391
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Yes, this sounds good to me, cc @SunMarc for the trainer
Yes, this could be cool! Maybe a bit of work necessary here, so could be cool to land it for RC1 rather than RC0.
No problem for me!
This is the only one I'm not sure about -> have smaller shards means that it's easier to parallelize load from checkpoints. Will let @ArthurZucker complete as he's been working with this quite a bit lately |
ArthurZucker
left a comment
ArthurZucker
left a comment
There was a problem hiding this comment.
have not really seen the exact line, but its something we do need to study: smaller shards -> better i/o file lock potentially?
I am probably wrong as we can parallel read a file (which we did not do before) so should be fine with main but let's test it out please
|
Thanks for the quick answers! I'll add 1. and 3. to this PR since it's small-ish and quite related. For 2. (safe serialization), I'll start a new PR later this week. For 4., I'd rather not touch it for now. Seems like an interesting topic but lower prio + requires investigation. |
Happy to have this, not sure we use |
|
Thanks for your feedback @LysandreJik @ArthurZucker @SunMarc ! I've addressed the remaining open questions so PR should now be in a final state for review :) If I see anything new, I'll do it in a separate PR. |
| _is_offline_mode = huggingface_hub.constants.HF_HUB_OFFLINE | ||
|
|
||
|
|
||
| def is_offline_mode(): | ||
| return _is_offline_mode | ||
| return constants.HF_HUB_OFFLINE | ||
|
|
||
|
|
||
| torch_cache_home = os.getenv("TORCH_HOME", os.path.join(os.getenv("XDG_CACHE_HOME", "~/.cache"), "torch")) | ||
| default_cache_path = constants.default_cache_path | ||
|
|
||
| # Determine default cache directory. Lots of legacy environment variables to ensure backward compatibility. | ||
| # Determine default cache directory. | ||
| # The best way to set the cache path is with the environment variable HF_HOME. For more details, check out this | ||
| # documentation page: https://huggingface.co/docs/huggingface_hub/package_reference/environment_variables. | ||
| # | ||
| # In code, use `HF_HUB_CACHE` as the default cache path. This variable is set by the library and is guaranteed | ||
| # to be set to the right value. | ||
| # | ||
| # TODO: clean this for v5? | ||
| PYTORCH_PRETRAINED_BERT_CACHE = os.getenv("PYTORCH_PRETRAINED_BERT_CACHE", constants.HF_HUB_CACHE) | ||
| PYTORCH_TRANSFORMERS_CACHE = os.getenv("PYTORCH_TRANSFORMERS_CACHE", PYTORCH_PRETRAINED_BERT_CACHE) | ||
| TRANSFORMERS_CACHE = os.getenv("TRANSFORMERS_CACHE", PYTORCH_TRANSFORMERS_CACHE) | ||
|
|
||
| HF_MODULES_CACHE = os.getenv("HF_MODULES_CACHE", os.path.join(constants.HF_HOME, "modules")) | ||
| TRANSFORMERS_DYNAMIC_MODULE_NAME = "transformers_modules" | ||
| SESSION_ID = uuid4().hex | ||
|
|
||
| # Add deprecation warning for old environment variables. | ||
| for key in ("PYTORCH_PRETRAINED_BERT_CACHE", "PYTORCH_TRANSFORMERS_CACHE", "TRANSFORMERS_CACHE"): | ||
| if os.getenv(key) is not None: | ||
| warnings.warn( | ||
| f"Using `{key}` is deprecated and will be removed in v5 of Transformers. Use `HF_HOME` instead.", | ||
| FutureWarning, | ||
| ) | ||
|
|
||
|
|
||
| S3_BUCKET_PREFIX = "https://s3.amazonaws.com/models.huggingface.co/bert" | ||
| CLOUDFRONT_DISTRIB_PREFIX = "https://cdn.huggingface.co" | ||
|
|
||
| _staging_mode = os.environ.get("HUGGINGFACE_CO_STAGING", "NO").upper() in ENV_VARS_TRUE_VALUES | ||
| _default_endpoint = "https://hub-ci.huggingface.co" if _staging_mode else "https://huggingface.co" | ||
|
|
||
| HUGGINGFACE_CO_RESOLVE_ENDPOINT = _default_endpoint | ||
| if os.environ.get("HUGGINGFACE_CO_RESOLVE_ENDPOINT", None) is not None: | ||
| warnings.warn( | ||
| "Using the environment variable `HUGGINGFACE_CO_RESOLVE_ENDPOINT` is deprecated and will be removed in " | ||
| "Transformers v5. Use `HF_ENDPOINT` instead.", | ||
| FutureWarning, | ||
| ) | ||
| HUGGINGFACE_CO_RESOLVE_ENDPOINT = os.environ.get("HUGGINGFACE_CO_RESOLVE_ENDPOINT", None) | ||
| HUGGINGFACE_CO_RESOLVE_ENDPOINT = os.environ.get("HF_ENDPOINT", HUGGINGFACE_CO_RESOLVE_ENDPOINT) | ||
| HUGGINGFACE_CO_PREFIX = HUGGINGFACE_CO_RESOLVE_ENDPOINT + "/{model_id}/resolve/{revision}/{filename}" | ||
| HUGGINGFACE_CO_EXAMPLES_TELEMETRY = HUGGINGFACE_CO_RESOLVE_ENDPOINT + "/api/telemetry/examples" | ||
|
|
| """ | ||
| try: | ||
| # Check if the model card is present on the remote repo | ||
| model_card = ModelCard.load(repo_id, token=token, ignore_metadata_errors=ignore_metadata_errors) |
There was a problem hiding this comment.
Are you sure we don't want to ignore metadata errors when it makes sense?
There was a problem hiding this comment.
Pretty sure I would say. To be honest I'm not sure anyone really used this parameter as it's hidden and even if you get an error, it doesn't tell you to use it. Also if a metadata error happens we won't be able to push the model card back to the Hub afterwards
|
[For maintainers] Suggested jobs to run (before merge) run-slow: conditional_detr, convnext, deformable_detr, dit, dpt, glpn, got_ocr2, groupvit, internvl, llama, maskformer |
Offline mode / env variables changes:
def offline_modehelper directly takes its value fromhuggingface_hub.constantswithout an intermediate_offline_modevariable. It looks like it was only useful for testing purposes. In practice it's much better to rely solely on the hfh's one so that if a user monkeypatch it, it will be globally monkeypatched both in huggingface_hub and transformers (and other libraries)huggingface_hub.constants.HF_HUB_CACHE. It is not possible anymore to define a cache specific totransformers(which is on-purpose^^)TRANSFORMERS_CACHEPYTORCH_TRANSFORMERS_CACHEPYTORCH_PRETRAINED_BERT_CACHEunittest.patch, less logging, usepytest.raises, etc.)HF_HUB_CACHE/HF_HOME/XDG_CACHE_HOMEtransformers.utils.hub.torch_cache_home(not used anywhere)transformers.utils.hub.default_cache_path(not used anywhere but exposed at root)transformers.utils.hub.HUGGINGFACE_CO_EXAMPLES_TELEMETRY(not used anywhere)transformers.utils.hub.HUGGINGFACE_CO_PREFIX(not used anywhere but exposed at root)transformers.utils.hub.HUGGINGFACE_CO_RESOLVE_ENDPOINT(used in some legacy parts and exposed at root, not used anymore)Basically, "do not parse high-level env variables from
transformers".PushToHubMixinchangesAlso some changes in
PushToHubMixin:organizationandrepo_urlfromPushToHubMixin._create_repoPushToHubMixin._create_repoentirely (it became just an alias so not worth it)organizationfromPushToHubMixinrepo_path_or_namefromPushToHubMixinignore_metadata_errorsfromPushToMixin. This parameter was added in #28405 but more as a copy-paste (see comment) rather than a deliberated decision. In practice if we ignore errors while loading the model card, we won't be able to push the card back to the Hub. Better to fail early here and not provide the option to fail later. (also removed fromcreate_and_tag_model_cardhelper)push_to_hubdo not use**kwargsanymore, remove it from the method signaturepush_to_hubkeyword-only to avoid confusion + reorder them. Onlyrepo_idcan be positional since it's the main arg. Same as what is done onhuggingface_hubside.use_temp_dirargument frompush_to_hub. We now use a tmp dir in all casesworking_or_temp_dirhelper (not used anymore)Open questions!
There are a few more things I would like to cleanup as well, either in this PR or a follow-up one. Not 100% sure though as one might disagree with me^^
push_to_hubshould not save to a local dir at all. Currently if the model name part ofrepo_idis also a local folder, then the model is saved in it and then pushed to the Hub, unlessuse_temp_dir=Trueis passed. This logic is super unintuitive in my opinion (and likely related to when we were pushing from a git repo). It also induce some logic to determine which files have been updated (and push only these ones). All in all quite a lot of complexity for little benefit. For the record, thePyTorchModelHubMixinalways use a tmp dirsafe_serialization=False? The v5 release feels a very good opportunity to definitely get rid of non-safetensors serialization. WDYT ?push_to_hub(...)all parameters are positional, meaning that the order counts, meaning it's harder to remove or add a new one. I would strongly suggest to use*,to explicitly require keyword-arguments. This is what we do inhuggingface_huband other places oftransformers. Let's take advantage of v5 to introduce this breaking change?max_shard_sizedefaults to 5GB. Recommendations are now to use <50GB chunks (the recommendation increased a lot since Xet is now the default). Should we increase the default value to e.g. 48GB? (not 50, to avoid having files slightly larger than 50).I'm willing to take care of all of these changes if approved. Sorry to bring this up so late in the release cycle. For some reasons I thought I had already taken a look at this but doesn't seem to be the case^^ Let me know @huggingface/transformers-core-maintainers