Skip to content

[Executorch] Add broadcasting support to optimized op_sub #8256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 33 commits into from
Feb 21, 2025

Conversation

kimishpatel
Copy link
Contributor

@kimishpatel kimishpatel commented Feb 6, 2025

Stack from ghstack (oldest at bottom):

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc @larryliu0820 @manuelcandales

Differential Revision: D69491818

Summary:
Refactoring broadcast handling utils that were added for op_mul. This is
in prepartion use these utils to handle broadcast for other ops such as
add, sub, div.
Plus remove a redundant test

Test Plan:
optimized_kernels_test in CI

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Summary:
This brings add op to feature parity, wrt, broadcasting, to mul op in
optimized kernels lib

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Summary:
This brings add op to feature parity, wrt, broadcasting, to mul op in
optimized kernels lib

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Summary:
Refactor op_add to conslidate commong broadcasting related improvements

Test Plan:
Previously added tests

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Feb 6, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit 6976e4c with merge base 2b81e6f (image):
💚 Looks good so far! There are no failures yet. 💚

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

…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@kimishpatel kimishpatel requested a review from swolchok February 6, 2025 06:41
@kimishpatel kimishpatel added module: kernels Issues related to kernel libraries and utilities, and code under kernels/ release notes: ops & kernels Changes to the opset and any new / changed kernel implementations labels Feb 7, 2025
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

[ghstack-poisoned]
TensorFactory<DTYPE> tf_a;

Tensor a =
tf_a.make({2, 2, 3}, /*data=*/{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12});
Copy link
Contributor

Choose a reason for hiding this comment

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

as before, programmatically filling test data would be nice I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I agree. Didnt quite occur to me at the time of writing but thanks for pointing out

@kimishpatel
Copy link
Contributor Author

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
@kimishpatel
Copy link
Contributor Author

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
@kimishpatel
Copy link
Contributor Author

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
@kimishpatel
Copy link
Contributor Author

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
@kimishpatel
Copy link
Contributor Author

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…ptimized op_sub"

Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
Summary:
This diff builds on top of previous one to add support for limited
handling of broadcasting for sub

Test Plan:
tests added

Reviewers:

Subscribers:

Tasks:

Tags:

cc larryliu0820 manuelcandales

Differential Revision: [D69491818](https://our.internmc.facebook.com/intern/diff/D69491818)

[ghstack-poisoned]
@kimishpatel
Copy link
Contributor Author

@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kimishpatel kimishpatel changed the base branch from gh/kimishpatel/156/base to main February 21, 2025 19:12
@kimishpatel kimishpatel merged commit 95caa8a into main Feb 21, 2025
48 of 50 checks passed
@kimishpatel kimishpatel deleted the gh/kimishpatel/156/head branch February 21, 2025 19:13
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. module: kernels Issues related to kernel libraries and utilities, and code under kernels/ release notes: ops & kernels Changes to the opset and any new / changed kernel implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants