Skip to content

[DataLoader2] Change graph traverse default option to only_datapipe=True#85654

Closed
ejguan wants to merge 1 commit intopytorch:masterfrom
ejguan:change_traverse_default
Closed

[DataLoader2] Change graph traverse default option to only_datapipe=True#85654
ejguan wants to merge 1 commit intopytorch:masterfrom
ejguan:change_traverse_default

Conversation

@ejguan
Copy link
Contributor

@ejguan ejguan commented Sep 26, 2022

Changes the default behavior of torch.utils.data.traverse function from only_datapipe=False to only_datapipe=True.

I am introducing this BC-breaking change for the following reasons:

  • Ultimately we want to remove only_datapipe and use it as only_datapipe=True. Keeping a default value as False makes the deprecation cycle even longer.
  • This function is tied to DataLoader2 graph. Since DataLoader2 is in prototype phase, any BC-breaking change is allowed. And, we do add a deprecation warning for removing the only_datapipe option.
  • I did a quick search on Github, 99% percent of usages are forks from torchdata or pytorch.

BC-breaking Note:

This PR changes the default option of torch.utils.data.traverse function to only_datapipe=True. When it traverses the DataPipe, it would skip all non-collection objects. This would reduce the traversing complexity and reduce the chance that traverse non-picklable objects.

@ejguan ejguan added release notes: dataloader release notes category topic: bc breaking topic category labels Sep 26, 2022
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 26, 2022

🔗 Helpful Links

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

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

✅ No Failures

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

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

@facebook-github-bot
Copy link
Contributor

@ejguan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@ejguan
Copy link
Contributor Author

ejguan commented Sep 27, 2022

Closing this PR by adopting #85667

@ejguan ejguan closed this Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants