-
Notifications
You must be signed in to change notification settings - Fork 282
Prepack conv weights #31
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
Conversation
f193736
to
379ddc1
Compare
I have a concern related but not specific to this PR w.r.t. the in-place changes of input data/weight tensors. PyTorch originally assumes the input tensors as constant and it is safe for re-entrance, e.g. called by multiple threads from JIT with fork and join. The in-place changes of these input tensors might break this assumption. Do we have the design to protect these in-place changes from multi-threaded access? |
As of right now, no. Modifying constant tensors on the fly, especially constant parameters, do have some implications we need to take care of. |
BTW, we are also implementing a standalone prepack jit pass. This PR is kind of a workaround to optimize the imperative path, so that we could achieve the same performance as the We may revert this design if customers are more inclined to use JIT, or if something goes wrong with it. |
Use cases for your consideration:
|
Thanks, Jiong. I discussed with Pinzhen, for case 1, we can capture |
a95469e
to
bf3a18e
Compare
We currently adopt prepacking conv weights in Pros:
Cons:
|
It will be a problem for conv too if we use winograd. Just FYI. |
m = orig_module_to(self, *args, **kwargs) | ||
|
||
device = torch._C._nn._parse_to(*args, **kwargs)[0] | ||
if device and device.type == 'dpcpp': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to check auto_dnnl here. If the user disables auto_dnnl, it should go through the original path.
No description provided.