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

Fix visualize_overlap for Inductor comm reordering #113066

Closed
wants to merge 2 commits into from

Conversation

yf225
Copy link
Contributor

@yf225 yf225 commented Nov 6, 2023

The following assumptions are not always valid and need checking:

  1. snode.node exists
  2. snode.node.layout.size exists
  3. snode.node.layout.stride exists
  4. snode.node.name exists

Also there is no guarantee that there won't be two collectives running at the same time. But it's hard to visualize the overlap in that case. So disable the visualization for that case for now.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @chenyang78 @kadeng @muchulee8 @aakhundov @ColinPeppler

@yf225 yf225 requested a review from wanchaol November 6, 2023 20:45
Copy link

pytorch-bot bot commented Nov 6, 2023

🔗 Helpful Links

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

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

✅ You can merge normally! (2 Unrelated Failures)

As of commit 051e856 with merge base fbafff3 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

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

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.

lgtm

@yf225
Copy link
Contributor Author

yf225 commented Nov 7, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 7, 2023
@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@yf225 yf225 added the topic: not user facing topic category label Nov 8, 2023
@yf225
Copy link
Contributor Author

yf225 commented Nov 8, 2023

@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

Skylion007 pushed a commit to Skylion007/pytorch that referenced this pull request Nov 14, 2023
The following assumptions are not always valid and need checking:
1. `snode.node` exists
2. `snode.node.layout.size` exists
3. `snode.node.layout.stride` exists
4. `snode.node.name` exists

Also there is no guarantee that there won't be two collectives running at the same time. But it's hard to visualize the overlap in that case. So disable the visualization for that case for now.

Pull Request resolved: pytorch#113066
Approved by: https://github.com/wanchaol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants