[Quant][core][gpu][improvement] Refactored implementation for conv2d_cudnn to use packed parameters (Reland PR#73510) #74220
Conversation
…cudnn to use packed parameters (Reland PR#73510) Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. [ghstack-poisoned]
CI Flow Status⚛️ CI FlowRuleset - Version:
|
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 1b65813 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
…cudnn to use packed parameters (Reland PR#73510) Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. ghstack-source-id: d37922b Pull Request resolved: #74220
|
@dzdang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| @@ -0,0 +1,125 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
nit: maybe just cudnn_utils.h?
There was a problem hiding this comment.
I remember we have fbgemm_utils.h and qnnpack_utils.h
There was a problem hiding this comment.
Yeah I can change it. In a later PR (https://github.com/pytorch/pytorch/pull/73959/files), I actually introduce another file utils.h file with a cudnn_utils namespace, but maybe I can change the naming convention on the later PR instead
There was a problem hiding this comment.
I see, I think it would make sense to put all utils in one place if possible
There was a problem hiding this comment.
Do you think it'd be better to just use utils.h isntead of cudnn_utils.h since everything is already inside the cudnn folder?
There was a problem hiding this comment.
yeah I feel that makes sense
…for conv2d_cudnn to use packed parameters (Reland PR#73510)" Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. We also renamed cudnnpack_utils.h to utils.h. Differential Revision: [D34886988](https://our.internmc.facebook.com/intern/diff/D34886988) [ghstack-poisoned]
…for conv2d_cudnn to use packed parameters (Reland PR#73510)" Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. We also renamed cudnnpack_utils.h to utils.h. Differential Revision: [D34886988](https://our.internmc.facebook.com/intern/diff/D34886988) [ghstack-poisoned]
…for conv2d_cudnn to use packed parameters (Reland PR#73510)" Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. We also renamed cudnnpack_utils.h to utils.h. Differential Revision: [D34886988](https://our.internmc.facebook.com/intern/diff/D34886988) [ghstack-poisoned]
…for conv2d_cudnn to use packed parameters (Reland PR#73510)" Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. We also renamed cudnnpack_utils.h to utils.h. Differential Revision: [D34886988](https://our.internmc.facebook.com/intern/diff/D34886988) [ghstack-poisoned]
|
@dzdang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
…cudnn to use packed parameters (Reland PR#73510) (#74220) Summary: Pull Request resolved: #74220 This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. Test Plan: Imported from OSS Reviewed By: jerryzh168 Differential Revision: D34886988 Pulled By: dzdang fbshipit-source-id: 9bdedcd6686956070cd1da8e76b69c0c76f34403
|
Hey @dzdang. |
|
Did we drop the |
|
I have to manually copy this and rename it to |
@jjsjann123 |
|
@malfet This PR is causing our cudnn v8 build to fail. One of the included files doesn't exist. #include <ATen/native/quantized/cudnn/cudnn_utils.h> |
@xwang223 @malfet |
|
This pull request has been reverted by 3fe1606. To re-land this change, please open another pull request, assignthe same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk). |
|
@xwang233 @jjsjann123 I reverted this PR. Master should be fine now when building with cudnn v8 |
…cudnn to use packed parameters (Reland PR#73510) (#74220) Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. [ghstack-poisoned]
…for conv2d_cudnn to use packed parameters (Reland PR#73510) (#74220)" Summary: This a reland of #73510 -- please look at this PR directly for a summary and test plan. The only change in this PR is we add the ops to check_forward_backward_compatibility.py to get around the backwards compatibility issue introduced in the previous PR that changes the name of the cudnn conv operations. [ghstack-poisoned]
|
This pull request has been reverted by 3fe1606. To re-land this change, please open another pull request, assignthe same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk). |
Stack from ghstack (oldest at bottom):
Summary:
This a reland of #73510 --
please look at this PR directly for a summary and test plan.
The only change in this PR is we add the ops to check_forward_backward_compatibility.py
to get around the backwards compatibility issue introduced in the previous PR that
changes the name of the cudnn conv operations. We also renamed cudnnpack_utils.h
to utils.h.
Differential Revision: D34886988