followup changes to allow unsupported datasets #261
Merged
Conversation
This file contains 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
[ghstack-poisoned]
tianyu-l
added a commit
that referenced
this pull request
Apr 24, 2024
ghstack-source-id: ce288a19c67fccd0751c6fd92ae14a161da8bfa3 Pull Request resolved: #261
facebook-github-bot
added
the
CLA Signed
fegin
reviewed
Apr 24, 2024
| raise ValueError( | ||
| f"Dataset {dataset_name} is not supported. " | ||
| f"Supported datasets are: {list(_supported_datasets.keys())}." | ||
| ) |
There was a problem hiding this comment.
Should we also error out if users pass both a supported dataset_name and dataset_path?
There was a problem hiding this comment.
Yeah I think this was a decision discussed long ago. The point is for any dataset, we'd like to offer two ways to use data, one is to download from HF hub, the other is to use local files if a path is provided. In the latter case, user still needs to be clear about the correspondence between dataset name and dataset path. Let's still keep this to make it less error-prone.
fegin
approved these changes
Apr 24, 2024
wanchaol
approved these changes
Apr 24, 2024
1. Remove `build_dataloader_fn` as we only use HF data loading for now. And it helps allow unsupported dataset if user specify a `dataset_path` 2. If user uses an unsupported `dataset` without specifying `dataset_path`, we should still throw. [ghstack-poisoned]
tianyu-l
added a commit
that referenced
this pull request
Apr 24, 2024
ghstack-source-id: 34b380d251e0a80ac5328fdaeb33a1e488f9c735 Pull Request resolved: #261
tianyu-l
added a commit
that referenced
this pull request
Apr 24, 2024
ghstack-source-id: 34b380d251e0a80ac5328fdaeb33a1e488f9c735 Pull Request resolved: #261
tianyu-l
added a commit
to tianyu-l/torchtitan_intern24
that referenced
this pull request
Aug 16, 2024
ghstack-source-id: 34b380d251e0a80ac5328fdaeb33a1e488f9c735 Pull Request resolved: pytorch#261
philippguevorguian
pushed a commit
to YerevaNN/YNNtitan
that referenced
this pull request
Aug 17, 2024
ghstack-source-id: 34b380d251e0a80ac5328fdaeb33a1e488f9c735 Pull Request resolved: pytorch#261
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stack from ghstack (oldest at bottom):
build_dataloader_fnas we only use HF data loading for now. And it helps allow unsupported dataset if user specify adataset_pathdatasetwithout specifyingdataset_path, we should still throw.