Repository navigation
[core] respect local_files_only=True when using sharded checkpoints - #12005
Conversation
This reverts commit 8d431dc.
|
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. |
|
|
||
| ignore_patterns = ["*.json", "*.md"] | ||
| # `model_info` call must guarded with the above condition. | ||
| model_files_info = model_info(pretrained_model_name_or_path, revision=revision, token=token) |
There was a problem hiding this comment.
So the purpose of this check is to verify if the necessary sharded files are present in the model repo before attempting a download, presumably to avoid a large download if all files aren't present. If we cannot connect to the hub, we just have to assume the necessary shard files are already present locally.
I think we can just skip this check if local_files_only=True and then check if all the shard filenames are present in the cached_folder
There was a problem hiding this comment.
I think just this is sufficient
if not local_files_only:
# run model_info check
Run snapshot download
Then after the cached_filenames is created, iterate over the files to verify they exist
for filename in cached_filename:
if not if not os.path.exists(filename):
raise EnvironmentError("expected file not present in {cached_folder}")
There was a problem hiding this comment.
- We don't have to run
snapshot_download()whenlocal_files_only=False, that might be unnecessary. - Why run
snapshot_download()after also runningmodel_info()? - Even if we run
snapshot_download()regardless oflocal_files_onlyvar, I think we should have it insidetry-exceptin case the endpoint cannot be pinged for some reason and raise theConnectionErroras before.
There was a problem hiding this comment.
Ah I see what you mean. Let me update. Sorry about the back and forth.
|
@DN6 see if the latest changes work for you. |
| cached_folder = os.path.join(cached_folder, subfolder) | ||
|
|
||
| # Check again after downloading/loading from the cache. | ||
| model_files_info = _get_filepaths_for_folder(cached_folder) |
There was a problem hiding this comment.
We don't need a new function to walk over the cached folder. If you just iterate over cached_filenames and check if the file exits. Avoid looping over files multiple times this way.
for cached_file in cached_filenames:
if not os.path.exists(cached_file):
raise EnvironmentError(f"{cached_file} not found in {cached_folder which is required..."
What does this PR do?
See: #11948