Update cache loading logic to check file existence before use #60
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
Hi, thanks for the pr. I agree the code here needs optimization; however, it is recommended to use cache when you are in ddp mode. This is because if use_cache is set to False in ddp, each of the processes will repeatedly cache the data, which may cause OOM. |
Author
|
Indeed, it is not appropriate to directly and roughly delete the is_ddp judgment. Now I have changed it to check whether the cache exists before using it to avoid using a non-existent cache. |
Previously, the code would automatically attempt to use cache files based on the is_ddp() condition and use_cache configuration. This led to failures when running with torchrun without existing cache files, as the program would try to load non-existent cache files in DDP mode. With this change, we modify the cache loading behavior to: 1. First check if cache files exist at the expected path 2. Only use cache files if they actually exist, regardless of DDP mode 3. Generate new cache files if they don't exist, even when use_cache=True This prevents torchrun from failing when cache files are not present, while still allowing efficient cache reuse when available. The behavior is now more predictable and robust, as it depends on actual file existence rather than implicit environment variables or assumptions about cache availability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the code would automatically attempt to use cache files in DDP mode (when is_ddp() returns True), which is detected by the presence of WORLD_SIZE environment variable set by torchrun. This caused failures when running with torchrun without existing cache files, as the program would try to load non-existent cache files.
With this change, cache usage is now solely determined by the use_cache configuration parameter, regardless of whether the code is running in DDP mode or not. This ensures that:
This prevents torchrun from failing when cache files are not present, making the cache behavior predictable and controlled by explicit configuration rather than implicit environment variables.