Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix periodic integration test and add helper message on torchdata import failure #353

Merged
merged 2 commits into from
May 22, 2024

Conversation

tianyu-l
Copy link
Contributor

@tianyu-l tianyu-l commented May 22, 2024

Stack from ghstack (oldest at bottom):

  1. use the same generic torch CI workflow for periodic integration test, as in Use torch generic workflow for CI, add ssh, artifacts #325 for cpu/gpu unit tests.
  2. StatefulDataloader is not in torchdata official release yet. Print helper message if user doesn't have a recent nightly installed.

tianyu-l added a commit that referenced this pull request May 22, 2024
…mport failure

ghstack-source-id: 1c2355c2d7d05f3118a8864ac996afe552fa3ff6
Pull Request resolved: #353
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 22, 2024
@tianyu-l tianyu-l requested a review from wconstab May 22, 2024 01:03
@tianyu-l tianyu-l changed the title fix i periodic integration test and add helper message on torchdata import failure fix periodic integration test and add helper message on torchdata import failure May 22, 2024
Copy link
Contributor

@wconstab wconstab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


try:
from torchdata.stateful_dataloader import StatefulDataLoader
except ModuleNotFoundError as e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe also include import error here incase user did not install torch data at all? For my case I had tried to install but the version was wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out ModuleNotFoundError is a subclass of ImportError, so changing to ImportError here.

…rchdata import failure"


1. use the same generic torch CI workflow for periodic integration test, as in #325 for cpu/gpu unit tests.
2. `StatefulDataloader` is not in `torchdata` official release yet. Print helper message if user doesn't have a recent nightly installed.

[ghstack-poisoned]
tianyu-l added a commit that referenced this pull request May 22, 2024
…mport failure

ghstack-source-id: 4db9ec111c83f7873253f19f0c95a997800e0f6b
Pull Request resolved: #353
@tianyu-l tianyu-l merged commit f6cc0e0 into gh/tianyu-l/13/base May 22, 2024
4 checks passed
tianyu-l added a commit that referenced this pull request May 22, 2024
…mport failure

ghstack-source-id: 4db9ec111c83f7873253f19f0c95a997800e0f6b
Pull Request resolved: #353
@tianyu-l tianyu-l deleted the gh/tianyu-l/13/head branch May 22, 2024 03:20
tianyu-l added a commit that referenced this pull request May 28, 2024
…mport failure

ghstack-source-id: 4db9ec111c83f7873253f19f0c95a997800e0f6b
Pull Request resolved: #353
tianyu-l added a commit to tianyu-l/torchtitan_intern24 that referenced this pull request Aug 16, 2024
…mport failure

ghstack-source-id: 4db9ec111c83f7873253f19f0c95a997800e0f6b
Pull Request resolved: pytorch#353
tianyu-l added a commit that referenced this pull request Aug 16, 2024
…mport failure

ghstack-source-id: 4db9ec111c83f7873253f19f0c95a997800e0f6b
Pull Request resolved: #353
philippguevorguian pushed a commit to YerevaNN/YNNtitan that referenced this pull request Aug 17, 2024
…mport failure

ghstack-source-id: 4db9ec111c83f7873253f19f0c95a997800e0f6b
Pull Request resolved: pytorch#353
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants