[DataLoader] Improve typing and documentation of Sampler API for customization #97338

stsouko · 2023-03-22T11:16:46Z

Explanation with examples of sampler customization added.

fixed TypeVar
removed unused init from Sampler class
added examples for custom sampler and batch sampler
Distributed sampler typing fixed.
_InfiniteConstantSampler fixed

Fixes #92268

removed init of _InfiniteConstantSampler

pytorch-bot · 2023-03-22T11:16:49Z

🔗 Helpful Links

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

📄 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 5585f89:
💚 Looks good so far! There are no failures yet. 💚

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

linux-foundation-easycla · 2023-03-22T11:16:50Z

The committers listed above are authorized under a signed CLA.

✅ login: stsouko / name: Ramil Nugmanov (b08c5b0, 322390c)

NivekT

Thanks for opening this PR! These cleanups are helpful and are always welcomed. Small comments below:

torch/utils/data/sampler.py

NivekT

Thanks! LGTM!

NivekT · 2023-03-23T21:27:45Z

@pytorchbot merge

pytorchmergebot · 2023-03-23T21:34:02Z

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 · 2023-03-23T21:44:13Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Lint / lintrunner / linux-job

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

NivekT

Actually some changes are needed (primarily linting):
https://github.com/pytorch/pytorch/actions/runs/4498692677/jobs/7931055601

torch/utils/data/sampler.py

DataLoader typing fixed.

NivekT

Thanks! Let me use CI to double check everything

torch/utils/data/sampler.py

NivekT

Sorry for the back-and-forth, but I'm unable to fix the remaining linting error through GitHub (i.e. remove Sequence from import in dataloader.py)

https://github.com/pytorch/pytorch/actions/runs/4515159665/jobs/7952316990?pr=97338

docstring fixed

stsouko · 2023-03-24T22:30:59Z

import fixed

torch/utils/data/sampler.py

NivekT · 2023-03-27T23:29:34Z

@pytorchbot merge

pytorchmergebot · 2023-03-27T23:32:12Z

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 · 2023-03-27T23:42:31Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

pull / linux-bionic-cuda11.7-py3.10-gcc7 / test (default, 3, 5, linux.4xlarge.nvidia.gpu)

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

NivekT · 2023-03-28T05:11:39Z

@pytorchbot merge

pytorchmergebot · 2023-03-28T05:13:26Z

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

ejguan

It seems that we need a follow-up PR to make sure it's not BC-breaking as Sampler might take indices other than integer

ejguan · 2023-04-17T18:55:21Z

torch/utils/data/dataloader.py

@@ -223,8 +217,8 @@ class DataLoader(Generic[T_co]):
    __initialized = False

    def __init__(self, dataset: Dataset[T_co], batch_size: Optional[int] = 1,
-                 shuffle: Optional[bool] = None, sampler: Union[Sampler, Iterable, None] = None,


Emmm. This is BC breaking. The given sampler isn't necessarily yielding integers.

ejguan · 2023-04-17T19:01:47Z

torch/utils/data/distributed.py


-
-class DistributedSampler(Sampler[T_co]):
+class DistributedSampler(Sampler[int]):


ejguan · 2023-04-17T19:02:33Z

torch/utils/data/sampler.py

@@ -12,23 +12,57 @@
    "WeightedRandomSampler",
 ]

-T_co = TypeVar('T_co', covariant=True)
+T_co = TypeVar('T_co', int, List[int], covariant=True)


This is BC-breaking as adding restriction on the expected type for Sampler.

API backward compatibility fixed: #97338 (comment) Mapped Dataset can accept noninteger indices from custom Samplers. Fixes #97338 Pull Request resolved: #100409 Approved by: https://github.com/ejguan, https://github.com/NivekT

kuraga · 2024-04-02T15:47:50Z

The data_source argument should be removed, but it hasn't been, #123172.

stsouko added 2 commits March 22, 2023 11:58

Sampler API described for customization.

b08c5b0

typing of dds fixed

322390c

removed init of _InfiniteConstantSampler

pytorch-bot bot added the release notes: dataloader release notes category label Mar 22, 2023

pytorchbot added the open source label Mar 22, 2023

zou3519 requested review from ejguan and NivekT March 22, 2023 16:37

zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Mar 22, 2023

NivekT reviewed Mar 22, 2023

View reviewed changes

torch/utils/data/sampler.py Outdated Show resolved Hide resolved

torch/utils/data/sampler.py Outdated Show resolved Hide resolved

better typing

aa223d8

stsouko requested review from NivekT and removed request for ejguan March 23, 2023 08:59

NivekT approved these changes Mar 23, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 23, 2023

NivekT suggested changes Mar 23, 2023

View reviewed changes

torch/utils/data/sampler.py Show resolved Hide resolved

deprecation warning added

9de1a86

DataLoader typing fixed.

stsouko requested a review from NivekT March 23, 2023 22:35

NivekT approved these changes Mar 24, 2023

View reviewed changes

torch/utils/data/sampler.py Outdated Show resolved Hide resolved

Update torch/utils/data/sampler.py

569ba58

NivekT reviewed Mar 24, 2023

View reviewed changes

torch/utils/data/sampler.py Outdated Show resolved Hide resolved

Update torch/utils/data/sampler.py

bb33a46

NivekT reviewed Mar 24, 2023

View reviewed changes

torch/utils/data/sampler.py Outdated Show resolved Hide resolved

Update torch/utils/data/sampler.py

db7c033

NivekT suggested changes Mar 24, 2023

View reviewed changes

removed unused import.

d4764a7

docstring fixed

stsouko requested a review from NivekT March 24, 2023 22:31

NivekT approved these changes Mar 27, 2023

View reviewed changes

torch/utils/data/sampler.py Outdated Show resolved Hide resolved

Update torch/utils/data/sampler.py

5585f89

pytorchmergebot added the Merged label Mar 28, 2023

pytorchmergebot closed this in 867b07b Mar 28, 2023

ejguan reviewed Apr 17, 2023

View reviewed changes

NivekT mentioned this pull request May 1, 2023

[DataLoader] __getitems__ added to description of Dataset API and better supported within Subset #100375

Closed

stsouko mentioned this pull request May 1, 2023

[DataLoader] Follow-up Fix: TypeVars of Sampler #100409

Closed

NivekT changed the title ~~Sampler API described for customization.~~ [DataLoader] Sampler API described for customization. May 3, 2023

NivekT changed the title ~~[DataLoader] Sampler API described for customization.~~ [DataLoader] Improve typing and documentation of Sampler API for customization May 3, 2023

kuraga mentioned this pull request Apr 2, 2024

torch.utils.data.Sampler: constructor's data_source argument should be removed #123172

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataLoader] Improve typing and documentation of Sampler API for customization #97338

[DataLoader] Improve typing and documentation of Sampler API for customization #97338

stsouko commented Mar 22, 2023

pytorch-bot bot commented Mar 22, 2023 •

edited

linux-foundation-easycla bot commented Mar 22, 2023 •

edited

NivekT left a comment

NivekT left a comment •

edited

NivekT commented Mar 23, 2023

pytorchmergebot commented Mar 23, 2023

pytorchmergebot commented Mar 23, 2023

NivekT left a comment

NivekT left a comment

NivekT left a comment

stsouko commented Mar 24, 2023

NivekT commented Mar 27, 2023

pytorchmergebot commented Mar 27, 2023

pytorchmergebot commented Mar 27, 2023

NivekT commented Mar 28, 2023

pytorchmergebot commented Mar 28, 2023

ejguan left a comment

ejguan Apr 17, 2023

ejguan Apr 17, 2023

ejguan Apr 17, 2023

kuraga commented Apr 2, 2024 •

edited



		class DistributedSampler(Sampler[T_co]):
		class DistributedSampler(Sampler[int]):

[DataLoader] Improve typing and documentation of Sampler API for customization #97338

[DataLoader] Improve typing and documentation of Sampler API for customization #97338

Conversation

stsouko commented Mar 22, 2023

pytorch-bot bot commented Mar 22, 2023 • edited

🔗 Helpful Links

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

✅ No Failures

linux-foundation-easycla bot commented Mar 22, 2023 • edited

NivekT left a comment

Choose a reason for hiding this comment

NivekT left a comment • edited

Choose a reason for hiding this comment

NivekT commented Mar 23, 2023

pytorchmergebot commented Mar 23, 2023

Merge started

pytorchmergebot commented Mar 23, 2023

Merge failed

NivekT left a comment

Choose a reason for hiding this comment

NivekT left a comment

Choose a reason for hiding this comment

NivekT left a comment

Choose a reason for hiding this comment

stsouko commented Mar 24, 2023

NivekT commented Mar 27, 2023

pytorchmergebot commented Mar 27, 2023

Merge started

pytorchmergebot commented Mar 27, 2023

Merge failed

NivekT commented Mar 28, 2023

pytorchmergebot commented Mar 28, 2023

Merge started

ejguan left a comment

Choose a reason for hiding this comment

ejguan Apr 17, 2023

Choose a reason for hiding this comment

ejguan Apr 17, 2023

Choose a reason for hiding this comment

ejguan Apr 17, 2023

Choose a reason for hiding this comment

kuraga commented Apr 2, 2024 • edited

pytorch-bot bot commented Mar 22, 2023 •

edited

linux-foundation-easycla bot commented Mar 22, 2023 •

edited

NivekT left a comment •

edited

kuraga commented Apr 2, 2024 •

edited