Conversation
Summary: If we have a permute -> unary chain -> mean, based on the reduction dims of the mean, we can either fully remove the preceding permute or move the permute after the mean. Case 1: Dims after permute are still in same order with respect to each other, we can fully get rid of the permute and just update the reduction dims of the mean. Case 2: Not case 1. In this case, it's better to move the permute after the mean, since the permute will operate on less data. Differential Revision: D102268214
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19103
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: ❌ 2 Cancelled Jobs, 2 Unrelated FailuresAs of commit fe0d927 with merge base c3f3d12 ( CANCELLED JOBS - The following jobs were cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@DrJessop has exported this pull request. If you are a Meta employee, you can view the originating Diff in D102268214. |
This PR needs a
|
|
@claude Review this PR |
|
Claude finished @mergennachin's task in 3m 44s —— View job Review of PR #19103: Remove or move permute after mean
SummaryThis PR adds Overall the code is correct and well-tested. I have a few observations and one potential issue below. Issues1. Potential issue with negative
The actual concern is the original No action needed — just noting for clarity. 2. Missing The Suggestions3. The The method 4. Consider a test with quantize/dequantize in the unary chain All current tests use 5. No test with negative reduction dims The code handles negative reduction dims via 6. Pass ordering in
VerdictThe logic is correct, the math for dim remapping and post-mean permutation computation checks out, and all test cases include numerical validation. The code is clean and follows existing patterns in the file. This looks good to land. |
Summary:
If we have a permute -> unary chain -> mean, based on the reduction dims of the mean, we can either fully remove the preceding permute or move the permute after the mean.
Case 1: Dims after permute are still in same order with respect to each other, we can fully get rid of the permute and just update the reduction dims of the mean.
Case 2: Not case 1. In this case, it's better to move the permute after the mean, since the permute will operate on less data.
Differential Revision: D102268214