Fix index out of bounds error for 1d convs #15995
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15995
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 1 New FailureAs of commit f578636 with merge base 12d17ef ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
bot
added
the
CLA Signed
Summary: Unfortunately, we are in a state where conv2d can run either 1d or 2d convs, and the padding/stride/dilation logic is pretty bad. Essentially, if we have a conv1d, we can produce a conv2d where stride becomes [1, value_provided] in the fusion_pass (see get_conv_args). That is why in ops_registrations, we have been indexing at 1. However, if we somehow construct a conv2d_nchw/nchw without going through that fusion_pass, this canonicalization is not done. Ideally, we have conv1d implementations where these arguments are scalars, not lists. But, this is a larger effort, since even the C++ kernels follow this convoluted logic. Temporary fix here is to index at -1, and we'll have to come back to cleaning up conv kernels. Reviewed By: hsharma35 Differential Revision: D87943382
DrJessop
force-pushed
the
export-D87943382
branch
from
bcd2d13 to
f578636
Compare
This PR needs a
|
Differential Revision: D87943382 Pull Request resolved: pytorch#15995
2 participants
Summary:
Unfortunately, we are in a state where conv2d can run either 1d or 2d convs, and the padding/stride/dilation logic is pretty bad.
Essentially, if we have a conv1d, we can produce a conv2d where stride becomes [1, value_provided] in the fusion_pass (see get_conv_args). That is why in ops_registrations, we have been indexing at 1. However, if we somehow construct a conv2d_nchw/nchw without going through that fusion_pass, this canonicalization is not done.
Ideally, we have conv1d implementations where these arguments are scalars, not lists. But, this is a larger effort, since even the C++ kernels follow this convoluted logic.
Temporary fix here is to index at -1, and we'll have to come back to cleaning up conv kernels.
Differential Revision: D87943382