Skip to content

Officially graduate ProtypeMPRS to MPRS#1009

Closed
ejguan wants to merge 1 commit intometa-pytorch:mainfrom
ejguan:export-D43245136
Closed

Officially graduate ProtypeMPRS to MPRS#1009
ejguan wants to merge 1 commit intometa-pytorch:mainfrom
ejguan:export-D43245136

Conversation

@ejguan
Copy link
Contributor

@ejguan ejguan commented Feb 13, 2023 •
edited
Loading

Summary:
Fixes: #932

  • Convert all references from ProtypeMPRS to MPRS
  • Remove usage of MPRS
  • Update colab
  • Add BC-breaking notes

Differential Revision: D43245136

BC-breaking Note:

PrototypeMultiProcessingReadingService replaces MultiProcessingReadingService.

0.5.0

MultiProcessingReadingService previously took the following arguments:

MultiProcessingReadingService(
    num_workers: int = 0,
    pin_memory: bool = False,
    timeout: float = 0,
    worker_init_fn: Optional[Callable[[int], None]] = None,
    multiprocessing_context=None,
    prefetch_factor: Optional[int] = None,
    persistent_workers: bool = False,
)

This PR

MultiProcessingReadingService takes the followng arguments:

MultiProcessingReadingService(
    num_workers: int = 0,
    multiprocessing_context: Optional[str] = None,
    worker_prefetch_cnt: int = 10,
    main_prefetch_cnt: int = 10,
    worker_init_fn: Optional[Callable[[DataPipe, WorkerInfo], DataPipe]] = None,
    worker_reset_fn: Optional[Callable[[DataPipe, WorkerInfo, SeedGenerator], DataPipe]] = None,
)

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Feb 13, 2023
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Feb 13, 2023

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

ejguan added a commit to ejguan/data that referenced this pull request Feb 13, 2023
@ejguan @facebook-github-bot
Officially graduate ProtypeMPRS to MPRS (meta-pytorch#1009)
f70b3ce 
Summary:
Pull Request resolved: meta-pytorch#1009

Fixes: meta-pytorch#932

- Convert all references from `ProtypeMPRS` to `MPRS`
- Remove usage of `MPRS`

Differential Revision: D43245136

fbshipit-source-id: eccfb9f31696a4f810a9f48dd5392e1802e0028a
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Feb 13, 2023

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

@ejguan
Copy link
Contributor Author

ejguan commented Feb 13, 2023

Needs to update colab after this PR has been landed.

@ejguan ejguan requested review from NivekT and wenleix February 13, 2023 22:17
NivekT
NivekT approved these changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@NivekT NivekT left a comment

Choose a reason for hiding this comment

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

Wooo! Gonna need a BC-breaking/feature announcement note for this one to inform users that the arguments have changed and the internals are different.

Also need to look closely at internal/external CIs.

wenleix
wenleix approved these changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@wenleix wenleix left a comment

Choose a reason for hiding this comment

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

Awesome!!!

test/dataloader2/test_dataloader2.py
self.assertEqual(batch, expected_batch)
expected_batch += 1

def test_dataloader2_multi_process_reading_service(self) -> None:
Copy link
Contributor

@wenleix wenleix Feb 13, 2023

Choose a reason for hiding this comment

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

do we have dataaloder 2 tests with new graduated MPRS? :)

Copy link
Contributor Author

@ejguan ejguan Feb 14, 2023

Choose a reason for hiding this comment

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

We do have all prototypeMPRS in test_dataloader2. I just rename them from prototypeMPRS to MPRS in the test file.

@ejguan
Copy link
Contributor Author

ejguan commented Feb 14, 2023

Wooo! Gonna need a BC-breaking/feature announcement note for this one to inform users that the arguments have changed and the internals are different.

Sounds good. Will add notes.

@ejguan ejguan added the topic: bc_breaking topic category label Feb 14, 2023
@ejguan @facebook-github-bot
Officially graduate ProtypeMPRS to MPRS (meta-pytorch#1009)
9707a13 
Summary:
Pull Request resolved: meta-pytorch#1009

Fixes: meta-pytorch#932

- Convert all references from `ProtypeMPRS` to `MPRS`
- Remove usage of `MPRS`

Reviewed By: wenleix, NivekT

Differential Revision: D43245136

fbshipit-source-id: 2195a7aae64b8ded5483f5b37383a4fa2d94f13e
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Feb 14, 2023

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

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Feb 14, 2023

This pull request has been merged in 73d3aa9.

facebook-github-bot pushed a commit to meta-pytorch/tnt that referenced this pull request Feb 14, 2023
@ejguan @facebook-github-bot
Officially graduate ProtypeMPRS to MPRS (#1009)
ad45d1a 
Summary:
X-link: meta-pytorch/data#1009

Fixes: meta-pytorch/data#932

- Convert all references from `ProtypeMPRS` to `MPRS`
- Remove usage of `MPRS`

Reviewed By: wenleix, NivekT

Differential Revision: D43245136

fbshipit-source-id: 1fd67f0e9d55a984209b8e58edecdb6070d919cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@wenleix wenleix wenleix approved these changes

@NivekT NivekT NivekT approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged topic: bc_breaking topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Start to graduate PrototypeMultiProcessingReadingService from "prototype mode"

4 participants

@ejguan @facebook-github-bot @wenleix @NivekT