Skip to content

[PP] Add spacing to visualizer#160474

Closed
H-Huang wants to merge 8 commits intogh/H-Huang/207/basefrom
gh/H-Huang/207/head
Closed

[PP] Add spacing to visualizer#160474
H-Huang wants to merge 8 commits intogh/H-Huang/207/basefrom
gh/H-Huang/207/head

Conversation

@H-Huang
Copy link
Member

@H-Huang H-Huang commented Aug 12, 2025

Stack from ghstack (oldest at bottom):

When visualizing the schedules using _PipelineScheduleExecution, we don't provide any spacing between dependencies, so when visualizing DualPipeV it looks like this:

image

While it has the correct order of operations, it does not show the dependencies correctly. As shown in the original implementation, it should look something like this:

image

This allows an option to add spacing to the visualizer, so it is easier to see dependencies. After change:

image

cc @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @pragupta @ezyang @msaroufim @dcci

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Aug 12, 2025

🔗 Helpful Links

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

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

⏳ No Failures, 3 Pending

As of commit 3ae84e6 with merge base 4840a1a (image):
💚 Looks good so far! There are no failures yet. 💚

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

H-Huang added a commit that referenced this pull request Aug 12, 2025
ghstack-source-id: a1fa129
Pull-Request: #160474
@pytorch-bot pytorch-bot bot added the oncall: distributed Add this issue/PR to distributed oncall triage queue label Aug 12, 2025
@H-Huang H-Huang added the release notes: distributed (pipeline) release notes category label Aug 12, 2025
[ghstack-poisoned]
H-Huang added a commit that referenced this pull request Aug 12, 2025
ghstack-source-id: 07a4671
Pull-Request: #160474
@H-Huang H-Huang requested review from kwen2501 and wconstab August 13, 2025 01:15
[ghstack-poisoned]
H-Huang added a commit that referenced this pull request Aug 13, 2025
ghstack-source-id: 517c54c
Pull-Request: #160474
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
H-Huang added a commit that referenced this pull request Aug 14, 2025
ghstack-source-id: 0fefbac
Pull-Request: #160474
@H-Huang H-Huang requested a review from fegin September 2, 2025 17:21
Copy link
Contributor

@fegin fegin left a comment

Choose a reason for hiding this comment

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

LGTM, I left some questions and suggestions.

[ghstack-poisoned]
H-Huang added a commit that referenced this pull request Sep 9, 2025
ghstack-source-id: f3bfd2a
Pull-Request: #160474
@H-Huang H-Huang added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 9, 2025
@H-Huang
Copy link
Member Author

H-Huang commented Sep 9, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

@ezyang
Copy link
Contributor

ezyang commented Sep 10, 2025

BTW, I notice this is not tested at all. I think there's a lot of value in figuring out how to test this, because this is the sort of code that bitrots quickly. If it is completely untestable, consider exporting a stable dump format (which you test against) and then just have the visualizer in a separate repo (easier to push changes async here too!)

markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
When visualizing the schedules using `_PipelineScheduleExecution`, we don't provide any spacing between dependencies, so when visualizing `DualPipeV` it looks like this:

<img width="3168" height="486" alt="image" src="https://github.com/user-attachments/assets/d2c881ad-4ee0-46b6-ac03-13e5600b5a55" />

While it has the correct order of operations, it does not show the dependencies correctly. As shown in the original implementation, it should look something like this:

<img width="3542" height="384" alt="image" src="https://github.com/user-attachments/assets/c930fa98-848e-4951-a58b-c81f41092d14" />

This allows an option to add spacing to the visualizer, so it is easier to see dependencies. After change:

<img width="3633" height="486" alt="image" src="https://github.com/user-attachments/assets/7708367e-bdb4-46e8-a7c4-f19e18047f59" />

Pull Request resolved: pytorch#160474
Approved by: https://github.com/fegin
mansiag05 pushed a commit to mansiag05/pytorch that referenced this pull request Sep 22, 2025
When visualizing the schedules using `_PipelineScheduleExecution`, we don't provide any spacing between dependencies, so when visualizing `DualPipeV` it looks like this:

<img width="3168" height="486" alt="image" src="https://github.com/user-attachments/assets/d2c881ad-4ee0-46b6-ac03-13e5600b5a55" />

While it has the correct order of operations, it does not show the dependencies correctly. As shown in the original implementation, it should look something like this:

<img width="3542" height="384" alt="image" src="https://github.com/user-attachments/assets/c930fa98-848e-4951-a58b-c81f41092d14" />

This allows an option to add spacing to the visualizer, so it is easier to see dependencies. After change:

<img width="3633" height="486" alt="image" src="https://github.com/user-attachments/assets/7708367e-bdb4-46e8-a7c4-f19e18047f59" />

Pull Request resolved: pytorch#160474
Approved by: https://github.com/fegin
cleonard530 pushed a commit to cleonard530/pytorch that referenced this pull request Sep 22, 2025
When visualizing the schedules using `_PipelineScheduleExecution`, we don't provide any spacing between dependencies, so when visualizing `DualPipeV` it looks like this:

<img width="3168" height="486" alt="image" src="https://github.com/user-attachments/assets/d2c881ad-4ee0-46b6-ac03-13e5600b5a55" />

While it has the correct order of operations, it does not show the dependencies correctly. As shown in the original implementation, it should look something like this:

<img width="3542" height="384" alt="image" src="https://github.com/user-attachments/assets/c930fa98-848e-4951-a58b-c81f41092d14" />

This allows an option to add spacing to the visualizer, so it is easier to see dependencies. After change:

<img width="3633" height="486" alt="image" src="https://github.com/user-attachments/assets/7708367e-bdb4-46e8-a7c4-f19e18047f59" />

Pull Request resolved: pytorch#160474
Approved by: https://github.com/fegin
dsashidh pushed a commit to dsashidh/pytorch that referenced this pull request Sep 26, 2025
When visualizing the schedules using `_PipelineScheduleExecution`, we don't provide any spacing between dependencies, so when visualizing `DualPipeV` it looks like this:

<img width="3168" height="486" alt="image" src="https://github.com/user-attachments/assets/d2c881ad-4ee0-46b6-ac03-13e5600b5a55" />

While it has the correct order of operations, it does not show the dependencies correctly. As shown in the original implementation, it should look something like this:

<img width="3542" height="384" alt="image" src="https://github.com/user-attachments/assets/c930fa98-848e-4951-a58b-c81f41092d14" />

This allows an option to add spacing to the visualizer, so it is easier to see dependencies. After change:

<img width="3633" height="486" alt="image" src="https://github.com/user-attachments/assets/7708367e-bdb4-46e8-a7c4-f19e18047f59" />

Pull Request resolved: pytorch#160474
Approved by: https://github.com/fegin
@github-actions github-actions bot deleted the gh/H-Huang/207/head branch October 11, 2025 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: distributed (pipeline) release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants