support ConvUnary in Inductor cpp wrapper#101392
support ConvUnary in Inductor cpp wrapper#101392chunyuan-w wants to merge 9 commits intogh/chunyuan-w/48/basefrom
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/101392
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 30a6d0b: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
ghstack-source-id: c6c2fc2 Pull Request resolved: pytorch#101392
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
jgong5
left a comment
jgong5
left a comment
There was a problem hiding this comment.
LGTM w/ some suggestions to simplify the code a bit.
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
|
|
||
| class OptionalAttr: | ||
| def __repr__(self): | ||
| return self.name |
There was a problem hiding this comment.
Base class should also contain self.name
torch/_inductor/graph.py
Outdated
| self.graph_id = graph_id | ||
| self.scheduler = None | ||
| self._warned_fallback = {"aten.convolution_backward"} | ||
| self.optional_variable = { |
There was a problem hiding this comment.
Emitted OPTIONAL_DECLARATION is cheap to compile, so I don't think it is necessary to record this. Also removing this will make may_convert_to_optional simpler.
There was a problem hiding this comment.
Removed the recording to simplify the code as suggested.
torch/_inductor/ir.py
Outdated
|
|
||
|
|
||
| def may_convert_to_optional(optional_value, value): | ||
| if not value and V.graph.cpp_wrapper: |
There was a problem hiding this comment.
not value -> value is not None?
There was a problem hiding this comment.
[] and "" are used here for empty value, thus I used not value to check it:
pytorch/torch/_inductor/mkldnn.py
Lines 76 to 87 in e9a7115
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
|
@desertfire I've addressed the above comments. Could you please help take another look? |
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
cc soumith voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]
|
@pytorchbot merge |
Merge startedYour 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 |
6 participants
Stack from ghstack (oldest at bottom):
cc @soumith @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @desertfire