[Executorch] Refactor op_mul's broadcasting utils #8204

kimishpatel · 2025-02-05T01:06:33Z

Stack from ghstack (oldest at bottom):

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:

cc @larryliu0820 @manuelcandales

Differential Revision: D69491816

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]

pytorch-bot · 2025-02-05T01:06:37Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

❌ 1 New Failure

As of commit c7f9e88 with merge base 5dd2ed3 ():

NEW FAILURE - The following job has failed:

Lint / lintrunner / linux-job (gh)
>>> Lint for backends/cadence/aot/export_example.py:

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

…g utils" 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: 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]

swolchok

I sure wish GitHub flagged copied/moved code for easier review

swolchok · 2025-02-06T18:47:55Z

kernels/optimized/cpu/op_mul.cpp

+    auto mul_lambda = [](auto x, auto y) { return x * y; };
+    return torch::executor::handle_broadcast_elementwise(
+        ctx, mul_lambda, a, b, out, selected_optimized_path);


nit: mul_lambda is std::multiplies, up to you which is more readable though

swolchok · 2025-02-07T16:08:08Z

kernels/optimized/cpu/binary_ops.h

+      "Failed to resize output tensor.");
+  const size_t outer_size = getLeadingDims(out, out.dim() - 1);
+  const auto broadcast_size = out.size(out.dim() - 1);
+  ET_SWITCH_REALB_TYPES(out_type, ctx, "mul.out", CTYPE, [&]() {


this isn't in mul anymore, but I see it's fixed in the next PR, close enough

…g utils" 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: cc larryliu0820 manuelcandales [ghstack-poisoned]

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: cc larryliu0820 manuelcandales [ghstack-poisoned]

…g utils" 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: cc larryliu0820 manuelcandales [ghstack-poisoned]

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: cc larryliu0820 manuelcandales [ghstack-poisoned]

swolchok

seems fine

swolchok · 2025-02-11T21:45:12Z

kernels/optimized/cpu/binary_ops.h

+  if ((selected_optimized_path ==
+       ElementwiseOptimizedPath::kBroadcastLastDim) ||
+      (selected_optimized_path ==
+       ElementwiseOptimizedPath::kBroadcastLastDimReverseArguments)) {


it occurs to me that we should separate the selected algorithm from whether to reverse arguments or not to make this read nicer, but that definitely doesn't have to go in this PR

kimishpatel · 2025-02-11T23:36:44Z

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

…g utils" 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: cc larryliu0820 manuelcandales Differential Revision: [D69491816](https://our.internmc.facebook.com/intern/diff/D69491816) [ghstack-poisoned]

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: cc larryliu0820 manuelcandales Differential Revision: [D69491816](https://our.internmc.facebook.com/intern/diff/D69491816) [ghstack-poisoned]

…g utils" 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: cc larryliu0820 manuelcandales Differential Revision: [D69491816](https://our.internmc.facebook.com/intern/diff/D69491816) [ghstack-poisoned]

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: cc larryliu0820 manuelcandales Differential Revision: [D69491816](https://our.internmc.facebook.com/intern/diff/D69491816) [ghstack-poisoned]

kimishpatel · 2025-02-12T17:32:19Z

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

…g utils" 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: cc larryliu0820 manuelcandales Differential Revision: [D69491816](https://our.internmc.facebook.com/intern/diff/D69491816) [ghstack-poisoned]

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: cc larryliu0820 manuelcandales Differential Revision: [D69491816](https://our.internmc.facebook.com/intern/diff/D69491816) [ghstack-poisoned]

kimishpatel · 2025-02-12T20:56:30Z

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

kimishpatel mentioned this pull request Feb 5, 2025

[ExecuTorch] Add broadcast support for optimized add op #8205

Merged

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 Feb 5, 2025

This was referenced Feb 6, 2025

[Executorch] Refactor op_add to support op_sub broadcasting #8255

Open

[Executorch] Add broadcasting support to optimized op_sub #8256

Open

[ExecuTorch] Add broadcasting support to optimized op_div #8257

Open

kimishpatel added 2 commits February 5, 2025 22:40

kimishpatel requested a review from swolchok February 6, 2025 06:40

swolchok approved these changes Feb 6, 2025

View reviewed changes

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

swolchok reviewed Feb 7, 2025

View reviewed changes

kimishpatel added 4 commits February 10, 2025 16:06

swolchok approved these changes Feb 11, 2025

View reviewed changes

github-actions bot mentioned this pull request Feb 11, 2025

Weekly pr metrics report - 2025-02-01..2025-02-07 wdvr/pytorch#6

Open

kimishpatel added 4 commits February 11, 2025 19:16

kimishpatel added 2 commits February 12, 2025 12:55

kimishpatel changed the base branch from gh/kimishpatel/153/base to main February 13, 2025 14:37

kimishpatel merged commit 6d3bf82 into main Feb 13, 2025
46 of 48 checks passed

kimishpatel deleted the gh/kimishpatel/153/head branch February 13, 2025 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Executorch] Refactor op_mul's broadcasting utils #8204

[Executorch] Refactor op_mul's broadcasting utils #8204

kimishpatel commented Feb 5, 2025 •

edited

Loading

pytorch-bot bot commented Feb 5, 2025 •

edited

Loading

swolchok left a comment

swolchok Feb 6, 2025

swolchok Feb 7, 2025

swolchok left a comment

swolchok Feb 11, 2025

kimishpatel commented Feb 11, 2025

kimishpatel commented Feb 12, 2025

kimishpatel commented Feb 12, 2025

[Executorch] Refactor op_mul's broadcasting utils #8204

[Executorch] Refactor op_mul's broadcasting utils #8204

Conversation

kimishpatel commented Feb 5, 2025 • edited Loading

pytorch-bot bot commented Feb 5, 2025 • edited Loading

🔗 Helpful Links

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

❌ 1 New Failure

swolchok left a comment

Choose a reason for hiding this comment

swolchok Feb 6, 2025

Choose a reason for hiding this comment

swolchok Feb 7, 2025

Choose a reason for hiding this comment

swolchok left a comment

Choose a reason for hiding this comment

swolchok Feb 11, 2025

Choose a reason for hiding this comment

kimishpatel commented Feb 11, 2025

kimishpatel commented Feb 12, 2025

kimishpatel commented Feb 12, 2025

kimishpatel commented Feb 5, 2025 •

edited

Loading

pytorch-bot bot commented Feb 5, 2025 •

edited

Loading