Support channels_last format in portable upsample kernels #9526
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GregoryComer
requested review from
JacobSzwejbka,
lucylq,
swolchok and
manuelcandales
as code owners
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9526
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit e01a442 with merge base 785afce ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D71690379 |
GregoryComer
added
module: kernels
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 23, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
1bfb530 to
fb62a3b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D71690379 |
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 23, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
fb62a3b to
e2f9e3b
Compare
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 23, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
e2f9e3b to
3b28a15
Compare
GregoryComer
added
the
release notes: ops & kernels
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 24, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
3b28a15 to
c8a3356
Compare
|
never worked with channels_last so probably not the right person to review this. ping me if I need to just learn it for this review |
GregoryComer
force-pushed
the
export-D71690379
branch
from
c8a3356 to
2f3bd98
Compare
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 25, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Differential Revision: D71690379
|
This pull request was exported from Phabricator. Differential Revision: D71690379 |
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 25, 2025
Summary: Pull Request resolved: pytorch#9526 Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
2f3bd98 to
4e4003f
Compare
|
does this have an affect on binary size? |
Good question. It should be pretty small, but I can run a BSB check internally to verify. |
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 25, 2025
Summary: Pull Request resolved: pytorch#9526 Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
4e4003f to
627f274
Compare
manuelcandales
approved these changes
Mar 25, 2025
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 26, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Reviewed By: manuelcandales Differential Revision: D71690379
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 26, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Reviewed By: manuelcandales Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
627f274 to
e059a8d
Compare
|
This pull request was exported from Phabricator. Differential Revision: D71690379 |
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. Reviewed By: manuelcandales Differential Revision: D71690379
GregoryComer
force-pushed
the
export-D71690379
branch
from
e059a8d to
e01a442
Compare
|
@GregoryComer has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
GregoryComer
added a commit
to GregoryComer/executorch
that referenced
this pull request
Mar 26, 2025
Summary: Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels. I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed. To accomplish this, this PR makes the following changes: - Update `check_upsample_2d_common_args` to relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input. - In the upsample kernels (bilinear and nearest), split out NCHW and NHWC variants. The NHWC variant interchanges the loop order as to maintain contiguous output accesses. - Add test coverage to ensure ATen numerical parity. cc larryliu0820 manuelcandales Test Plan: ``` buck test executorch/kernels/test: ``` Reviewed By: manuelcandales Differential Revision: D71690379 Pulled By: GregoryComer
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Support channels_last input format in portable CPU upsample_bilinear2d and upsample_nearest2d kernels. This is useful for resize-in-model patterns when the user wants to pass inputs in channels_last format. It also (theoretically) allows for more effective auto-vectorization when vectorizing along the channels dim when there are a larger number of channels.
I considered generalizing the kernel to handle arbitrary dim order, but having a specialized channels last version allows for traversing the output in contiguous order. I could add a separate, arbitrarily-strided variant, but we can take that as a follow-up if needed.
To accomplish this, this PR makes the following changes:
check_upsample_2d_common_argsto relax the dim order restriction. It now allows for both default and channels_last dim order and verifies that the output dim order matches the input.Differential Revision: D71690379
cc @larryliu0820 @manuelcandales