Skip to content

Conversation

GregoryComer
Copy link
Member

Summary:
Models with a batch norm following a conv3d cause an internal error during lowering. This diff fixes it by updating the partitioning logic to only rely on fusion with 1d and 2d convs.

This is because XNNPACK doesn't currently support standalone batch norms and only partitions norms that can be fused. We can't fuse with Conv3d, because XNNPACK doesn't have an implementation. The partitioner constraint was missing logic to exclude Conv3d.

Differential Revision: D81069236

Copy link

pytorch-bot bot commented Aug 26, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13696

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 Cancelled Jobs, 37 Pending, 3 Unrelated Failures

As of commit 68486af with merge base 00b50b2 (image):

CANCELLED JOBS - The following jobs were cancelled. Please retry:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 26, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D81069236

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Aug 26, 2025
Summary:

Models with a batch norm following a conv3d cause an internal error during lowering. This diff fixes it by updating the partitioning logic to only rely on fusion with 1d and 2d convs.

This is because XNNPACK doesn't currently support standalone batch norms and only partitions norms that can be fused. We can't fuse with Conv3d, because XNNPACK doesn't have an implementation. The partitioner constraint was missing logic to exclude Conv3d.

Differential Revision: D81069236
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D81069236

@GregoryComer GregoryComer added the release notes: none Do not include this in the release notes label Aug 26, 2025
@GregoryComer GregoryComer marked this pull request as draft August 29, 2025 21:14
Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Stamping to unblock.

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Sep 9, 2025
Summary:

Models with a batch norm following a conv3d cause an internal error during lowering. This diff fixes it by updating the partitioning logic to only rely on fusion with 1d and 2d convs.

This is because XNNPACK doesn't currently support standalone batch norms and only partitions norms that can be fused. We can't fuse with Conv3d, because XNNPACK doesn't have an implementation. The partitioner constraint was missing logic to exclude Conv3d.

Differential Revision: D81069236
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D81069236

GregoryComer added a commit to GregoryComer/executorch that referenced this pull request Sep 9, 2025
Summary:

Models with a batch norm following a conv3d cause an internal error during lowering. This diff fixes it by updating the partitioning logic to only rely on fusion with 1d and 2d convs.

This is because XNNPACK doesn't currently support standalone batch norms and only partitions norms that can be fused. We can't fuse with Conv3d, because XNNPACK doesn't have an implementation. The partitioner constraint was missing logic to exclude Conv3d.

Differential Revision: D81069236
Summary:

Models with a batch norm following a conv3d cause an internal error during lowering. This diff fixes it by updating the partitioning logic to only rely on fusion with 1d and 2d convs.

This is because XNNPACK doesn't currently support standalone batch norms and only partitions norms that can be fused. We can't fuse with Conv3d, because XNNPACK doesn't have an implementation. The partitioner constraint was missing logic to exclude Conv3d.

Differential Revision: D81069236
@GregoryComer GregoryComer marked this pull request as ready for review September 9, 2025 00:48
@GregoryComer GregoryComer added this to the 1.0.0 milestone Sep 9, 2025
@GregoryComer
Copy link
Member Author

CI failures are infra related or broken on trunk.

@facebook-github-bot
Copy link
Contributor

@GregoryComer has imported this pull request. If you are a Meta employee, you can view this in D81069236.

@GregoryComer GregoryComer merged commit d05a793 into pytorch:main Sep 9, 2025
269 of 276 checks passed
mergennachin added a commit to mergennachin/executorch-1 that referenced this pull request Sep 10, 2025
Summary:
Forward pyre fix

https://www.internalfb.com/diff/D81069236
pytorch#13696

bypass-github-export-checks
bypass-github-pytorch-ci-checks
bypass-github-executorch-ci-checks

Reviewed By: digantdesai

Differential Revision: D82118651
mergennachin added a commit that referenced this pull request Sep 10, 2025
Summary:
Forward pyre fix

https://www.internalfb.com/diff/D81069236
#13696

bypass-github-export-checks
bypass-github-pytorch-ci-checks
bypass-github-executorch-ci-checks

Reviewed By: digantdesai

Differential Revision: D82118651
StrycekSimon pushed a commit to nxp-upstream/executorch that referenced this pull request Sep 23, 2025
…ytorch#14170)

Summary:
Forward pyre fix

https://www.internalfb.com/diff/D81069236
pytorch#13696

bypass-github-export-checks
bypass-github-pytorch-ci-checks
bypass-github-executorch-ci-checks

Reviewed By: digantdesai

Differential Revision: D82118651
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported release notes: none Do not include this in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants