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

[tests] add generic size/type test for all ProcessGroups #32

Closed
d4l3k opened this issue Dec 11, 2024 · 2 comments · Fixed by #45
Closed

[tests] add generic size/type test for all ProcessGroups #32

d4l3k opened this issue Dec 11, 2024 · 2 comments · Fixed by #45
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@d4l3k
Copy link
Member

d4l3k commented Dec 11, 2024

We have quite a few different ProcessGroup implementations in https://github.com/pytorch-labs/torchft/blob/main/torchft/process_group.py

We want a standardized test helper to test world_size 1 behavior to ensure that all common/inputs have the right sizes/types.

Key requirements:

  • We want to test all of the collectives supported by torchft.ProcessGroup https://github.com/pytorch-labs/torchft/blob/main/torchft/process_group.py#L88
  • Should run realistic input types (float32, bfloat16?) and verify shapes/types of outputs
  • We want to test the output behavior is correct both for Work.wait() and Work.get_future().
  • Can call it multiple times with different configurations
  • Optional: support world size >1

Add / update tests in https://github.com/pytorch-labs/torchft/blob/main/torchft/process_group_test.py

Example test:

def test_error_swallowing_process_group(self) -> None:
    pg = ErrorSwallowingProcessGroupWrapper(DummyProcessGroup(0, 1))

    self._test_pg(pg)

    pg.report_error(RuntimeException("some error"))

    # ensure that the default values after an error have the right shapes/types
    self._test_pg(pg)
@d4l3k d4l3k added good first issue Good for newcomers enhancement New feature or request labels Dec 11, 2024
@Krishn1412
Copy link

Hey @d4l3k , can I pick this up?

@d4l3k
Copy link
Member Author

d4l3k commented Dec 12, 2024

@Krishn1412 I had someone in mind already for this issue -- would you be interested in taking on #36 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants