Skip to content

Break flag bc#3128

Closed
mcr229 wants to merge 1 commit intopytorch:mainfrom
mcr229:export-D56271242
Closed

Break flag bc#3128
mcr229 wants to merge 1 commit intopytorch:mainfrom
mcr229:export-D56271242

Conversation

@mcr229
Copy link
Contributor

@mcr229 mcr229 commented Apr 18, 2024

Summary:
XNNPACK broke Backwards Compatibility by forcing pooling operations to reduce dims, and introducing a flag to allow these operation to keep the dims.

This is backwards breaking because previously XNNPACK would not keep the dims if no flag was given, now a flag must be specified to keep the dims. While initially we proposed the inverse to maintain backwards compatibility, they have encountered breakages and have decided to commit to this breakage. As we are a downstream dependency, and will have to accept this breakages ourselves, it is important that we break early before this is used in any production code.

As a result we break BC Here by accepting XNNPACK's change.

git diff roll_back_commit > rollback.patch
cd fbsource/fbcode/xplat/third-party/XNNPACK/XNNPACK
git apply ../../../../../../rollback.patch

Differential Revision: D56271242

@pytorch-bot
Copy link

pytorch-bot bot commented Apr 18, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 0a79fd1 with merge base fff20a7 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot facebook-github-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 Apr 18, 2024
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@mcr229 mcr229 force-pushed the export-D56271242 branch from 1369ea0 to d235464 Compare May 8, 2024 20:20
mcr229 added a commit to mcr229/executorch that referenced this pull request May 8, 2024
Summary:
Pull Request resolved: pytorch#3128

XNNPACK broke Backwards Compatibility by forcing pooling operations to reduce dims, and introducing a flag to allow these operation to keep the dims.

This is backwards breaking because previously XNNPACK would not keep the dims if no flag was given, now a flag must be specified to keep the dims. While initially we proposed the inverse to maintain backwards compatibility, they have encountered breakages and have decided to commit to this breakage. As we are a downstream dependency, and will have to accept this breakages ourselves, it is important that we break early before this is used in any production code.

As a result we break BC Here by accepting XNNPACK's change.

```
git diff roll_back_commit > rollback.patch
cd fbsource/fbcode/xplat/third-party/XNNPACK/XNNPACK
git apply ../../../../../../rollback.patch
```

We have to update the change in ExecuTorch, as a result we change the XNNPACK dep we are pointing to the branch containing these changes here:
https://github.com/digantdesai/XNNPACK/commits/et_v21/

Reviewed By: digantdesai

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

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

@mcr229 mcr229 force-pushed the export-D56271242 branch from d235464 to ca9dc00 Compare May 9, 2024 17:04
mcr229 added a commit to mcr229/executorch that referenced this pull request May 9, 2024
Summary:
Pull Request resolved: pytorch#3128

XNNPACK broke Backwards Compatibility by forcing pooling operations to reduce dims, and introducing a flag to allow these operation to keep the dims.

This is backwards breaking because previously XNNPACK would not keep the dims if no flag was given, now a flag must be specified to keep the dims. While initially we proposed the inverse to maintain backwards compatibility, they have encountered breakages and have decided to commit to this breakage. As we are a downstream dependency, and will have to accept this breakages ourselves, it is important that we break early before this is used in any production code.

As a result we break BC Here by accepting XNNPACK's change.

```
git diff roll_back_commit > rollback.patch
cd fbsource/fbcode/xplat/third-party/XNNPACK/XNNPACK
git apply ../../../../../../rollback.patch
```

We have to update the change in ExecuTorch, as a result we change the XNNPACK dep we are pointing to the branch containing these changes here:
https://github.com/digantdesai/XNNPACK/commits/et_v21/

Reviewed By: digantdesai

Differential Revision: D56271242
Summary:
Pull Request resolved: pytorch#3128

XNNPACK broke Backwards Compatibility by forcing pooling operations to reduce dims, and introducing a flag to allow these operation to keep the dims.

This is backwards breaking because previously XNNPACK would not keep the dims if no flag was given, now a flag must be specified to keep the dims. While initially we proposed the inverse to maintain backwards compatibility, they have encountered breakages and have decided to commit to this breakage. As we are a downstream dependency, and will have to accept this breakages ourselves, it is important that we break early before this is used in any production code.

As a result we break BC Here by accepting XNNPACK's change.

```
git diff roll_back_commit > rollback.patch
cd fbsource/fbcode/xplat/third-party/XNNPACK/XNNPACK
git apply ../../../../../../rollback.patch
```

We have to update the change in ExecuTorch, as a result we change the XNNPACK dep we are pointing to the branch containing these changes here:
https://github.com/digantdesai/XNNPACK/commits/et_v21/

Reviewed By: digantdesai

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

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

@mcr229 mcr229 force-pushed the export-D56271242 branch from ca9dc00 to 0a79fd1 Compare May 9, 2024 17:10
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 6c56122.

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 Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants