ENH: Add runtime CPU dispatching support for einsum #17107
Conversation
This patch doesn't cause any performance changes, it just aims to simplify the review process for numpy#17049, according to numpy#17049 (comment)
|
/cc, @eric-wieser |
|
This is still a lot more than I was hoping for. I'll have a go myself, and if it proves too challenging then probably approve this. |
|
Done in #17109. I think this should be easy to reapply over the top, assuming that PR passes CI (Local builds are having cython issues for me) |
| #include <numpy/halffloat.h> | ||
| #include <npy_pycompat.h> | ||
|
|
||
| #include <ctype.h> |
There was a problem hiding this comment.
I don't think it makes sense to fill this file with includes like this one. I've tried to address this in #17109.
There was a problem hiding this comment.
looks good to me, a private header shared between two sources, maybe I should rename it to einsum_private.h
| @@ -853,6 +853,7 @@ def get_mathlib_info(*args): | |||
| join('src', 'multiarray', 'dragon4.c'), | |||
| join('src', 'multiarray', 'dtype_transfer.c'), | |||
| join('src', 'multiarray', 'einsum.c.src'), | |||
| join('src', 'multiarray', 'einsum.dispatch.c.src'), | |||
There was a problem hiding this comment.
Headers need to be listed in this file too
There was a problem hiding this comment.
since einsum_helpers.h is a private header only used by einsum.c.src and einsum.dispatch.c.src
There was a problem hiding this comment.
The fact it's private isn't relevant from what I remember: headers should be in the list above so that rebuilds can detect they've changed. See my PR for details.
There was a problem hiding this comment.
thank you for the clarification
|
Cancelled travis to save 2.5 hours of CI, can restart it if after closing or merging #17109 |
It will not gonna make that difference, except for missing including the helper header into the dependencies. however, I don't mind to re-work this pr after merging #17109. After all, I just wanna speed up the process of merging #17049. |
|
closed in favor of #17109 |
ENH: Add runtime CPU dispatching support for einsum
This patch doesn't cause any performance changes,
it just aims to simplify the review process for #17049,
according to #17049 (comment)