MAINT: Split einsum into multiple files #17109
Conversation
1aa04c5 to
c3067fb
Compare
|
@seiko2plus: I named the file |
c3067fb to
b4ef35f
Compare
|
@eric-wieser, |
|
Thanks! That confirms to me that the file shouldn't be renamed until your follow-up PR which enables dispatching. |
There was a problem hiding this comment.
there's no need for two pull-requests, the following changes should add support for runtime CPU dispatching. few extra changes will need to be done by #17049 during the rebase.
| @@ -0,0 +1,1897 @@ | |||
| /* | |||
There was a problem hiding this comment.
rename it to einsum.dispatch.c.src
| * | ||
| * See LICENSE.txt for the license. | ||
| */ | ||
|
|
There was a problem hiding this comment.
| /* @targets baseline sse2 */ |
| /* None of the above specializations caught it, general loops */ | ||
| return _unspecialized_table[type_num][nop <= 3 ? nop : 0]; | ||
| } | ||
| #include "einsum_sumprod.h" |
There was a problem hiding this comment.
just combine the two headers into a one
EDIT: it's okay to keep them as-is I just don't see a reason for it.
There was a problem hiding this comment.
One header describes what is in the sumprod.c file, the other is unrelated debug macros that we should probably just remove in future.
In my mind there is - the point is to review code changes seperately to code moves. Doling both at the same time results in unreviewable PRs. |
|
@eric-wieser, I mean having this pr and #17107. |
Putting the sumprod stuff in a separate file makes a handful of open PRs easier to review. The only changes to the large diffs here are: * Some defines and typedefs moved to headers * `NPY_VISIBILITY_HIDDEN` was added to the one function used from `sumprod`
b4ef35f to
f7df11c
Compare
|
Splitting up is definitely nicer, I admit I often struggle with breaking up changes that are interconnected into steps, but here it is indeed a clear first step. Thanks Eric! |
Putting the sumprod stuff in a separate file makes a handful of open PRs easier to review.
The only changes to the large diffs here are:
NPY_NO_EXPORTwas added to the one function used fromsumprodThis is split from #17107 and #17049, which mixed this copy-paste with other review-worthy code changes.