-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
ENH, TST: Bring the NumPy C SIMD vectorization interface "NPYV" to Python #16782
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
Conversation
@seiko2plus Can you rebase this off master now that #16397 is merged? |
3496af1
to
bd00f82
Compare
@mattip, The binary size of this module depending on the default value of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, thanks.
circleCI breakage is fixed on master, so this needs a rebase/merge to clear it. |
2bc08a8
to
0c06bd6
Compare
done, the circleCI should pass it. |
0c06bd6
to
6801b58
Compare
Can you think of a way to redo |
@mattip, yes but I will need c++(template) or python through pyas_template.py that's how the pyas template looks loops_cmp.dispatch.pyas.c. currently working on simplifying it. I will move it later into a separate pr along with doc and testing unit. |
6801b58
to
b59d8ae
Compare
it seems I have to remove the draft status to make Travis and Shippable fetching it |
I think we should merge this to unlock the other PRs. It still needs docstrings, but let's leave that for a future PR. @eric-wieser what do you think? |
I am happy to leave the documentation for another PR, which may involve some UX changes. For instance, in order to use this I must do
The size of the module is not too bad for the boost in functionality: it allows trying out all the SIMD intrinsics via python. |
The code for the new modules looks good in this patch, as do the tests that those modules behave as expected. I haven't reviewed the SIMD tests in this PR that use the new modules to actually do the testing in any detail, but someone else can sign off on those. Alternatively, we could split them to a new PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TestUtility
class seems a bit odd to me. It might make more sense as a standalone class that uses instance state - the suggestions below are incomplete but should give an overall view of what I'm suggesting.
Its tempting to make SuffixInfo
class I suggest be part of the _simd
module, rather than just in this one test.
'_simd' is a new module to bring the NumPy C SIMD vectorization interface "NPYV" The module is designed to be extremely flexible so that it can accommodate any kind intrinsics, also to generate a python interface almost similar to the C interface. The main purpose of this module is to test NPYV intrinsics in python, but still can be used as an effective solution in designing SIMD kernels. Also add a new command-line argument `--simd-test` to control of targeted CPU features for the `_simd` module. Co-authored-by: Matti Picus <matti.picus@gmail.com> Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
…rectly Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
- use plain variables - clean up aligned allocate - use `PyArg_ParseTuple` for empty args - use `Py_ssize_t` instead of `unsigned` and `size_t` - improve coding style - no need for a custom raises assertions - use parametrize instead of inner loops - leave a comment about nature of mode testing unit - shift to get max/min of int72 - add more info to repr of vector object - get ride of exec() and use type() instead - use `.inc` as extension for sub-headers instead of `.h` - add `FMA4` and drop `SSE41` from _SIMD targets Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
49c3f49
to
5d8c3be
Compare
@seiko2plus there are a couple of suggestion still open. Could you either accept them or comment? This is really close now, lets finish it up. |
@mattip, both comments were part of |
Thanks @seiko2plus and @eric-wieser. Please open new issues/PR for parts of this that still need iterating on, it is a large addition so there is bound to be more to finish. |
Implement new CPython extension(_simd module)
A module to bring the NumPy C SIMD vectorization interface "NPYV"
The module is designed to be extremely flexible so that it can accommodate any kind
intrinsics, also to generate a python interface almost similar to the C interface.
The main purpose of this module is to test NPYV intrinsics in python,
but still can be used as an effective solution in designing SIMD kernels.
Also, add a new command-line argument
--simd-test
to control of targetedCPU features for the
_simd
module.Add testing unit that covers the current implemented intrinsics
based on _simd module.
TODO:
follow NumPy/Python coding stylefollow @eric-wieser suggestion for naming conventionsDisabled the following tasks, since I'm not sure which pull-request is going to merge first
[ ] add test unit for math intrinsics(sqrt, square, abs, recip) #16247