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.

Sign up for GitHub

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

Jump to bottom

updated test cases to use MultithreadTestCase #108749

Closed
wants to merge 2 commits into from
Closed

updated test cases to use MultithreadTestCase #108749

wants to merge 2 commits into from

Conversation

SaumyaBhushan
Copy link

workerB
Fixes #108744

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 7, 2023 •
edited

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 5 New Failures

As of commit 586c12d with merge base fae9547 (image):

NEW FAILURES - The following jobs have failed:

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 7, 2023 •
edited

CLA Signed

The committers listed above are authorized under a signed CLA.

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Sep 7, 2023
@zou3519 zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 8, 2023
@SaumyaBhushan
Copy link
Author

Hi @zou3519 @rohan-varma @H-Huang Please review this PR

wanchaol
wanchaol requested changes Sep 8, 2023
View reviewed changes
Copy link
Contributor

@wanchaol wanchaol left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! a few comments inlined.

test/distributed/_tensor/test_matrix_ops.py
skip_unless_torch_gpu,
with_comms,
)


class DistMatrixOpsTest(DTensorTestBase):
class DistMatrixOpsTest(DTensorOpTestBase):
@with_comms
def test_addmm(self):
device_mesh = DeviceMesh(self.device_type, list(range(self.world_size)))
Copy link
Contributor

Choose a reason for hiding this comment

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

can we also change the device_mesh init to use self.build_mesh, thanks!

test/distributed/_tensor/test_tensor_ops.py
with_comms,
)


class DistTensorOpsTest(DTensorTestBase):
class DistTensorOpsTest(DTensorOpTestBase):
@with_comms
def test_aten_contiguous(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, let's use build_mesh instead.

test/distributed/_tensor/test_math_ops.py
skip_unless_torch_gpu,
with_comms,
)


class DistMathOpsTest(DTensorTestBase):
class DistMathOpsTest(DTensorOpTestBase):
@with_comms
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need the with_comms wrapper when switching to OpTestBase, please fix those

test/distributed/_tensor/test_matrix_ops.py
skip_unless_torch_gpu,
with_comms,
)


class DistMatrixOpsTest(DTensorTestBase):
class DistMatrixOpsTest(DTensorOpTestBase):
@with_comms
Copy link
Contributor

Choose a reason for hiding this comment

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

same, can remove with_comms

test/distributed/_tensor/test_tensor_ops.py
with_comms,
)


class DistTensorOpsTest(DTensorTestBase):
class DistTensorOpsTest(DTensorOpTestBase):
@with_comms
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 7, 2023

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Nov 7, 2023
@github-actions github-actions bot closed this Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wanchaol wanchaol wanchaol requested changes

@mrshenli mrshenli Awaiting requested review from mrshenli

@zhaojuanmao zhaojuanmao Awaiting requested review from zhaojuanmao

@rohan-varma rohan-varma Awaiting requested review from rohan-varma

@H-Huang H-Huang Awaiting requested review from H-Huang

@awgu awgu Awaiting requested review from awgu

@kwen2501 kwen2501 Awaiting requested review from kwen2501

@fegin fegin Awaiting requested review from fegin

@fduwjj fduwjj Awaiting requested review from fduwjj

@wz337 wz337 Awaiting requested review from wz337

Assignees
No one assigned
Labels
open source Stale topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

switch more test cases to use MultithreadTestCase
4 participants
@SaumyaBhushan @wanchaol @zou3519 @pytorchbot