executorch icon indicating copy to clipboard operation
executorch copied to clipboard

Refactor binary op partitioner configs under binary op config class

Open mcr229 opened this issue 8 months ago • 8 comments

Problem

A lot of duplicated code is done for our partitioner configs which take in two inputs (mul, add, sub, etc.) I believe we can refactor these configs: https://github.com/pytorch/executorch/blob/1a9a59b94491f03620dce622fee922c1d2bc1fa7/backends/xnnpack/partition/config/generic_node_configs.py#L100-L107

so that they all inherit from a parent BinaryConfig class. We can then enforce common constraints for these binary configs. This is similar to what we do with the GEMM Config:

https://github.com/pytorch/executorch/blob/1a9a59b94491f03620dce622fee922c1d2bc1fa7/backends/xnnpack/partition/config/gemm_configs.py#L48

Verification

Make sure all existing CI is passing. python -m unittest executorch.backends.xnnpack.test

Resources

https://discord.gg/a9r5KZDNfZ

cc @digantdesai @cbilgin

mcr229 avatar Mar 06 '25 20:03 mcr229

Can I take on this issue?

AlexFierro9 avatar Mar 22 '25 16:03 AlexFierro9

@AlexFierro9 feel free to assign it to yourself and tag me here if you have any questions

mcr229 avatar Mar 24 '25 18:03 mcr229

@mcr229 Hi, I want to try to take on this issue, but it seems like I don't have the permission to assign it to myself. Could you please give me some help please. Thanks

csc010228 avatar Mar 25 '25 05:03 csc010228

Done!

mcr229 avatar Mar 25 '25 19:03 mcr229

Thank you very much! I am working on it

csc010228 avatar Mar 26 '25 01:03 csc010228

Hi @csc010228, thanks for picking up the task! Checking in, do you need any help on the issue? Feel free to ask here or on the discord.

lucylq avatar Apr 09 '25 16:04 lucylq

@csc010228 what is the status on the issue?

metascroy avatar Apr 22 '25 20:04 metascroy

@lucylq Hi, I am still reading the project's code and understanding its logic, and I have joined the discord channel.

csc010228 avatar Apr 23 '25 05:04 csc010228

Hello @csc010228, just wanted to check in on the progress of this issue. Thanks in advance!

SS-JIA avatar May 21 '25 16:05 SS-JIA