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

Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable #109869

Closed

Conversation

…VTs instead of ConstantVariable

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 22, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit 804f719 with merge base 691f8ca (image):
💚 Looks good so far! There are no failures yet. 💚

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

… requisite VTs instead of ConstantVariable"

cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Sep 22, 2023
…VTs instead of ConstantVariable

ghstack-source-id: a389af0b267967429a71c4c5b559c2895d67d012
Pull Request resolved: #109869

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable
@voznesenskym voznesenskym added ciflow/trunk Trigger trunk jobs on your pull request topic: not user facing topic category labels Sep 22, 2023
@ezyang
Copy link
Contributor

ezyang commented Sep 22, 2023

Can you do the ConstantVariable.create change on its own? I'll stamp it. (BTW, if you don't want to update the call sites, you can technically make the constructor do what you want by defining __new__ on ConstantVariable lol)

… requisite VTs instead of ConstantVariable"

cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
@voznesenskym voznesenskym changed the title [WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable Sep 22, 2023
…site VTs instead of ConstantVariable"

cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
@voznesenskym
Copy link
Collaborator Author

Can you do the ConstantVariable.create change on its own? I'll stamp it. (BTW, if you don't want to update the call sites, you can technically make the constructor do what you want by defining __new__ on ConstantVariable lol)

no __new__ please. Also .create is a well known pattern in dynamo. Also sure, can make the change on its own.

…site VTs instead of ConstantVariable"

cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Sep 22, 2023
…VTs instead of ConstantVariable

ghstack-source-id: 3f6b203548f29c1f4b07b1941b4a499348772768
Pull Request resolved: #109869

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable
@voznesenskym
Copy link
Collaborator Author

@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

pytorchmergebot added a commit that referenced this pull request Sep 23, 2023
…site VTs instead of ConstantVariable"

cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
…site VTs instead of ConstantVariable"

cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Sep 24, 2023
…VTs instead of ConstantVariable

ghstack-source-id: 13571a067511cb802d35f4e5ce003cc56bf9692b
Pull Request resolved: #109869

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

more test

more test
@voznesenskym
Copy link
Collaborator Author

@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

# getattr(torch.ops, "torchvision", None), "roi_align"
# )
# except ImportError:
return False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Staged the wrong file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, weird

…site VTs instead of ConstantVariable"

cc penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov

[ghstack-poisoned]
voznesenskym added a commit that referenced this pull request Sep 24, 2023
…VTs instead of ConstantVariable

ghstack-source-id: 662add5b03d3167a515cba3e96c154455aaba1bd
Pull Request resolved: #109869

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

[WIP] Better invariants - always route list/tuple to their requisite VTs instead of ConstantVariable

more test

more test

more test
@voznesenskym
Copy link
Collaborator Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team Raised by workflow job

@pytorchmergebot
Copy link
Collaborator

The merge job was canceled. If you believe this is a mistake, then you can re trigger it through pytorch-bot.

@voznesenskym
Copy link
Collaborator Author

@pytorchbot merge

@voznesenskym
Copy link
Collaborator Author

The CI fiasco on this pr has been vaguely awful

@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

@facebook-github-bot facebook-github-bot deleted the gh/voznesenskym/227/head branch September 27, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants