Refactor sharding data pipe into a seperate file#94095

Closed

wenleix wants to merge 1 commit intopytorch:masterfrom

wenleix:export-D43014692

Contributor

wenleix commented Feb 4, 2023 •

edited

Loading

Move ShardingFilterIterDataPipe into a dedicated file.

Also, propose to have a dedicated parent class (_ShardingIterDataPipe) for sharding data pipe, as this seems more like a "system/engine-level" datapipe that gives strong hints to RS on how to execute, and needs first-class citizen treatment in RS (compared with other "user-level" datapipe that are mostly composable Callable[[Iterable], Iterable]. So we don't need to based on whether is_shardable and apply_sharding are presented in DataPipe in graph_settings.py. But open to other discussions.

Open question: Should
ShardingRoundRobinDispatcherIterDataPipe also be considered as a _ShardingIterDataPipe? (e.g. this sharding is executed by replicating (the metadata), while ShardingRoundRobinDispatcherIterDataPipe hints too expensive to replicate so requires round robin data exchange/dispatch).

Differential Revision: D43014692

pytorch-bot bot commented Feb 4, 2023 •

edited

Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/94095

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 11e365b:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

pytorch-bot bot added the release notes: dataloader label

linux-foundation-easycla bot commented Feb 4, 2023 •

edited

Loading

The committers listed above are authorized under a signed CLA.

✅ login: wenleix / name: Wenlei Xie (f4ed8a498e9e4856ac8661e6cda59a357403a5bf)

facebook-github-bot added the fb-exported label

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix changed the title ~~[RFC] Refactor ShardingFilterIterDataPipe into a seperate file~~ [WIP, RFC] Refactor ShardingFilterIterDataPipe into a seperate file

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from 73b1dc2 to d20f165 Compare

February 4, 2023 00:29

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from d20f165 to f4ed8a4 Compare

February 4, 2023 00:37

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from f4ed8a4 to 9243a21 Compare

February 4, 2023 00:44

wenleix force-pushed the export-D43014692 branch from 9243a21 to e430106 Compare

February 4, 2023 01:01

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from e430106 to 08480eb Compare

February 4, 2023 05:52

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

1 similar comment

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from 08480eb to 73f02d9 Compare

February 4, 2023 05:59

wenleix force-pushed the export-D43014692 branch from 73f02d9 to 2e3e805 Compare

February 4, 2023 06:32

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from 2e3e805 to 18deef7 Compare

February 4, 2023 06:44

Contributor

facebook-github-bot commented Feb 4, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from 0a89bb2 to ca08787 Compare

February 7, 2023 02:20

Contributor

facebook-github-bot commented Feb 7, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from ca08787 to c63b86d Compare

February 7, 2023 03:39

Contributor

facebook-github-bot commented Feb 7, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from c63b86d to 4c25ed3 Compare

February 7, 2023 04:18


          Refactor ShardingFilterIterDataPipe into a seperate file (pytorch#987)

11e365b

Summary:
X-link: meta-pytorch/data#987

Pull Request resolved: pytorch#94095

Differential Revision:
D43014692

Move `ShardingFilterIterDataPipe` into a dedicated file.

Also, propose to have a dedicated parent class (`_ShardingIterDataPipe`) for sharding data pipe, as this seems more like a "system/engine-level" datapipe that gives strong hints to RS on how to execute, and needs first-class citizen treatment in RS (compared with other "user-level" datapipe that are mostly composable `Callable[[Iterable], Iterable]`.  But open to other discussions.

### Open question
Should
[ShardingRoundRobinDispatcherIterDataPipe](https://github.com/pytorch/data/blob/01fc76200354501b057bb439b43a1f05f609dd0a/torchdata/datapipes/iter/util/sharding.py#L16-L17) also be considered as a `_ShardingIterDataPipe`? (e.g. this sharding is executed by replicating (the metadata), while `ShardingRoundRobinDispatcherIterDataPipe` hints too expensive to replicate so requires round robin data exchange/dispatch).

D43014692

Test Plan:
sandcastle and CI

How to run unit tests in buck related to such changes? :)

Reviewed By: seemethere

fbshipit-source-id: fcd2a4e57b15fdf7411c959a38c377ac114f0ecb

Contributor

facebook-github-bot commented Feb 7, 2023

This pull request was exported from Phabricator. Differential Revision: D43014692

wenleix force-pushed the export-D43014692 branch from 4c25ed3 to 11e365b Compare

February 7, 2023 05:13

facebook-github-bot pushed a commit to meta-pytorch/data that referenced this pull request


          Refactor ShardingFilterIterDataPipe into a seperate file (#987)

914d3dd

Summary:
Pull Request resolved: #987

X-link: pytorch/pytorch#94095

Differential Revision:
D43014692

Move `ShardingFilterIterDataPipe` into a dedicated file.

Also, propose to have a dedicated parent class (`_ShardingIterDataPipe`) for sharding data pipe, as this seems more like a "system/engine-level" datapipe that gives strong hints to RS on how to execute, and needs first-class citizen treatment in RS (compared with other "user-level" datapipe that are mostly composable `Callable[[Iterable], Iterable]`.  But open to other discussions.

### Open question
Should
[ShardingRoundRobinDispatcherIterDataPipe](https://github.com/pytorch/data/blob/01fc76200354501b057bb439b43a1f05f609dd0a/torchdata/datapipes/iter/util/sharding.py#L16-L17) also be considered as a `_ShardingIterDataPipe`? (e.g. this sharding is executed by replicating (the metadata), while `ShardingRoundRobinDispatcherIterDataPipe` hints too expensive to replicate so requires round robin data exchange/dispatch).

D43014692

Reviewed By: seemethere

fbshipit-source-id: b5f2f6d8b3574d0d98b22ca55134c143786c7c2c

Contributor

facebook-github-bot commented Feb 7, 2023

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

pytorch-bot bot added the ciflow/trunk label

Collaborator

pytorchmergebot commented Feb 7, 2023

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot added the Merged label

pytorchmergebot closed this in

d6dec1a

wenleix deleted the export-D43014692 branch

February 7, 2023 21:26

wenleix mentioned this pull request

Continue support sharding pipes in tud.datapipes.iter.grouping as deprecated #94527

Closed

kiukchung mentioned this pull request

(bug)(torch2.0/datapipes) Potentially backwards incompatible change with DataLoader and is_shardable Datapipes #96975

Closed

kanpapa mentioned this pull request

Error during build of Horizon BPU (cross-compilation) runtime wenet-e2e/wenet#2639

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk fb-exported Merged release notes: dataloader