[functional collective] change the Python APIs to only use the native funcol ops #123777

yifuwang · 2024-04-10T22:07:19Z

Stack from ghstack (oldest at bottom):

-> [functional collective] change the Python APIs to only use the native funcol ops #123777
[functional_collective] remove the logic that forces torch-xla to use legacy funcol #123776

Summary

After this PR, the functional collective Python APIs will stop honoring TORCH_DISABLE_NATIVE_FUNCOL and only use native funcol ops. Specifically, this PR:

Removed use_native_funcol().
Removed the code path in the Python APIs when use_native_funcol() is False.
Changed the CI tests that runs on both native funcol and legacy funcol through the Python API to only run with native funcol.

Test Changes

test_functional_api.py

Removed the tests where only one of output_split_sizes or input_split_sizes is specified. This behavior is unreliable has been removed from the native funcol.

Removed TestWaitiness which tests an implementation detail of the legacy funcol. We have equivalent tests for native funcol in test/distributed/test_c10d_functional_native.py

pytorch/test/distributed/test_c10d_functional_native.py

Lines 114 to 116 in b7fac76

    
           assert not output.completed 
        
           assert output.eq(expect).all() 
        
           assert output.completed

test/distributed/_tensor/test_dtensor.py
test/distributed/_tensor/test_dtensor_compile.py
test/distributed/test_device_mesh.py
test/distributed/_tensor/experimental/test_tp_transform.py
test/distributed/_tensor/test_matrix_ops.py
test/distributed/test_inductor_collectives.py

All these tests were double running with both native funcol and legacy funcol. Changed to only run with native funcol.

test/distributed/test_c10d_functional_native.py

Removed the run_with_native_funcol decorators.

cc @mrshenli @pritamdamania87 @zhaojuanmao @satgera @rohan-varma @gqchen @aazzolini @osalpekar @jiayisuse @H-Huang @kwen2501 @awgu @penguinwu @fegin @XilunWu @wanchaol @fduwjj @wz337 @tianyu-l @wconstab @yf225 @chauhang

[ghstack-poisoned]

pytorch-bot · 2024-04-10T22:07:22Z

🔗 Helpful Links

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

📄 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 655f2ba with merge base 585cd11 ():
💚 Looks good so far! There are no failures yet. 💚

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

cc mrshenli pritamdamania87 zhaojuanmao satgera rohan-varma gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu penguinwu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab yf225 chauhang [ghstack-poisoned]

ghstack-source-id: 0653689 Pull Request resolved: #123777

… the native funcol ops" ## Summary After this PR, the functional collective Python APIs will stop honoring `TORCH_DISABLE_NATIVE_FUNCOL` and only use native funcol ops. Specifically, this PR: - Removed `use_native_funcol()`. - Removed the code path in the Python APIs when `use_native_funcol()` is `False`. - Changed the CI tests that runs on both native funcol and legacy funcol through the Python API to only run with native funcol. ## Test Changes `test_functional_api.py` - Removed the tests where only one of output_split_sizes or input_split_sizes is specified. This behavior is unreliable has been removed from the native funcol. - Removed `TestWaitiness` which tests an implementation detail of the legacy funcol. We have equivalent tests for native funcol in `test/distributed/test_c10d_functional_native.py` https://github.com/pytorch/pytorch/blob/b7fac76fc259394136bc77b3e39d5705919e5c4c/test/distributed/test_c10d_functional_native.py#L114-L116 `test/distributed/_tensor/test_dtensor.py` `test/distributed/_tensor/test_dtensor_compile.py` `test/distributed/test_device_mesh.py` `test/distributed/_tensor/experimental/test_tp_transform.py` `test/distributed/_tensor/test_matrix_ops.py` `test/distributed/test_inductor_collectives.py` - All these tests were double running with both native funcol and legacy funcol. Changed to only run with native funcol. `test/distributed/test_c10d_functional_native.py` - Removed the `run_with_native_funcol` decorators. cc mrshenli pritamdamania87 zhaojuanmao satgera rohan-varma gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu penguinwu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab yf225 chauhang [ghstack-poisoned]

… funcol ops ghstack-source-id: 2c2fe59 Pull Request resolved: #123777

wanchaol

Awesome to see this happens!

yifuwang · 2024-04-12T17:03:57Z

@pytorchbot merge

pytorchmergebot · 2024-04-12T17:05:42Z

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 · 2024-04-12T20:05:24Z

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x ede1ca6ebf63c15b052d1b8fb5efb1359c553e41 returned non-zero exit code 1

Auto-merging test/distributed/test_functional_api.py
CONFLICT (content): Merge conflict in test/distributed/test_functional_api.py
Auto-merging torch/distributed/_functional_collectives.py
error: could not apply ede1ca6ebf6... [functional collective] change the Python APIs to only use the native funcol ops
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

Details for Dev Infra team

Raised by workflow job

… the native funcol ops" ## Summary After this PR, the functional collective Python APIs will stop honoring `TORCH_DISABLE_NATIVE_FUNCOL` and only use native funcol ops. Specifically, this PR: - Removed `use_native_funcol()`. - Removed the code path in the Python APIs when `use_native_funcol()` is `False`. - Changed the CI tests that runs on both native funcol and legacy funcol through the Python API to only run with native funcol. ## Test Changes `test_functional_api.py` - Removed the tests where only one of output_split_sizes or input_split_sizes is specified. This behavior is unreliable has been removed from the native funcol. - Removed `TestWaitiness` which tests an implementation detail of the legacy funcol. We have equivalent tests for native funcol in `test/distributed/test_c10d_functional_native.py` https://github.com/pytorch/pytorch/blob/b7fac76fc259394136bc77b3e39d5705919e5c4c/test/distributed/test_c10d_functional_native.py#L114-L116 `test/distributed/_tensor/test_dtensor.py` `test/distributed/_tensor/test_dtensor_compile.py` `test/distributed/test_device_mesh.py` `test/distributed/_tensor/experimental/test_tp_transform.py` `test/distributed/_tensor/test_matrix_ops.py` `test/distributed/test_inductor_collectives.py` - All these tests were double running with both native funcol and legacy funcol. Changed to only run with native funcol. `test/distributed/test_c10d_functional_native.py` - Removed the `run_with_native_funcol` decorators. cc mrshenli pritamdamania87 zhaojuanmao satgera rohan-varma gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu penguinwu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab yf225 chauhang [ghstack-poisoned]

… funcol ops ghstack-source-id: df89ecc Pull Request resolved: #123777

yifuwang · 2024-04-13T00:01:13Z

@pytorchbot merge

pytorchmergebot · 2024-04-13T00:03:03Z

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

… funcol ops (pytorch#123777) ## Summary After this PR, the functional collective Python APIs will stop honoring `TORCH_DISABLE_NATIVE_FUNCOL` and only use native funcol ops. Specifically, this PR: - Removed `use_native_funcol()`. - Removed the code path in the Python APIs when `use_native_funcol()` is `False`. - Changed the CI tests that runs on both native funcol and legacy funcol through the Python API to only run with native funcol. ## Test Changes `test_functional_api.py` - Removed the tests where only one of output_split_sizes or input_split_sizes is specified. This behavior is unreliable has been removed from the native funcol. - Removed `TestWaitiness` which tests an implementation detail of the legacy funcol. We have equivalent tests for native funcol in `test/distributed/test_c10d_functional_native.py` https://github.com/pytorch/pytorch/blob/b7fac76fc259394136bc77b3e39d5705919e5c4c/test/distributed/test_c10d_functional_native.py#L114-L116 `test/distributed/_tensor/test_dtensor.py` `test/distributed/_tensor/test_dtensor_compile.py` `test/distributed/test_device_mesh.py` `test/distributed/_tensor/experimental/test_tp_transform.py` `test/distributed/_tensor/test_matrix_ops.py` `test/distributed/test_inductor_collectives.py` - All these tests were double running with both native funcol and legacy funcol. Changed to only run with native funcol. `test/distributed/test_c10d_functional_native.py` - Removed the `run_with_native_funcol` decorators. Pull Request resolved: pytorch#123777 Approved by: https://github.com/wanchaol ghstack dependencies: pytorch#123776

WIP

4a5c6ec

[ghstack-poisoned]

yifuwang mentioned this pull request Apr 10, 2024

[functional_collective] remove the logic that forces torch-xla to use legacy funcol #123776

Closed

pytorch-bot bot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Apr 10, 2024

yifuwang requested review from wanchaol and wconstab April 10, 2024 22:12

yifuwang marked this pull request as ready for review April 10, 2024 22:12

yifuwang marked this pull request as draft April 10, 2024 22:13

yifuwang removed request for wanchaol and wconstab April 10, 2024 22:13

Update on "WIP"

16a7c12

cc mrshenli pritamdamania87 zhaojuanmao satgera rohan-varma gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu penguinwu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab yf225 chauhang [ghstack-poisoned]

pytorch-bot bot added ciflow/inductor release notes: distributed (c10d) release notes category labels Apr 10, 2024

Update on "WIP"

40ae016

cc mrshenli pritamdamania87 zhaojuanmao satgera rohan-varma gqchen aazzolini osalpekar jiayisuse H-Huang kwen2501 awgu penguinwu fegin XilunWu wanchaol fduwjj wz337 tianyu-l wconstab yf225 chauhang [ghstack-poisoned]

yifuwang pushed a commit that referenced this pull request Apr 10, 2024

WIP

cee2c8c

ghstack-source-id: 0653689 Pull Request resolved: #123777

yifuwang changed the title ~~WIP~~ [functional collective] change the Python API to only use the native funcol ops Apr 11, 2024

yifuwang changed the title ~~[functional collective] change the Python API to only use the native funcol ops~~ [functional collective] change the Python APIs to only use the native funcol ops Apr 11, 2024

yifuwang marked this pull request as ready for review April 11, 2024 21:14

yifuwang requested review from wanchaol and wconstab April 11, 2024 21:14

yifuwang requested a review from yf225 April 11, 2024 21:15

yifuwang pushed a commit that referenced this pull request Apr 11, 2024

[functional collective] change the Python APIs to only use the native…

ede1ca6

… funcol ops ghstack-source-id: 2c2fe59 Pull Request resolved: #123777

wanchaol approved these changes Apr 12, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Apr 12, 2024

pytorchmergebot added the merging label Apr 12, 2024

pytorchmergebot removed the merging label Apr 12, 2024

yifuwang pushed a commit that referenced this pull request Apr 13, 2024

[functional collective] change the Python APIs to only use the native…

3d36f33

… funcol ops ghstack-source-id: df89ecc Pull Request resolved: #123777

yifuwang added the topic: not user facing topic category label Apr 13, 2024

pytorchmergebot added the merging label Apr 13, 2024

pytorchmergebot added the Merged label Apr 13, 2024

pytorchmergebot closed this in 2a2e1d8 Apr 13, 2024

pytorchmergebot removed the merging label Apr 13, 2024

github-actions bot deleted the gh/yifuwang/78/head branch May 14, 2024 01:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[functional collective] change the Python APIs to only use the native funcol ops #123777

[functional collective] change the Python APIs to only use the native funcol ops #123777

Uh oh!

yifuwang commented Apr 10, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Apr 10, 2024 •

edited

Loading

Uh oh!

wanchaol left a comment

Uh oh!

yifuwang commented Apr 12, 2024

Uh oh!

pytorchmergebot commented Apr 12, 2024

Uh oh!

pytorchmergebot commented Apr 12, 2024

Uh oh!

yifuwang commented Apr 13, 2024

Uh oh!

pytorchmergebot commented Apr 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	assert not output.completed
	assert output.eq(expect).all()
	assert output.completed

[functional collective] change the Python APIs to only use the native funcol ops #123777

[functional collective] change the Python APIs to only use the native funcol ops #123777

Uh oh!

Conversation

yifuwang commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Changes

Uh oh!

pytorch-bot bot commented Apr 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

wanchaol left a comment

Choose a reason for hiding this comment

Uh oh!

yifuwang commented Apr 12, 2024

Uh oh!

pytorchmergebot commented Apr 12, 2024

Merge started

Uh oh!

pytorchmergebot commented Apr 12, 2024

Merge failed

Uh oh!

yifuwang commented Apr 13, 2024

Uh oh!

pytorchmergebot commented Apr 13, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yifuwang commented Apr 10, 2024 •

edited

Loading

pytorch-bot bot commented Apr 10, 2024 •

edited

Loading