test code for user defined types in numpy
Hello, As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here: https://github.com/girving/rational The tests run under either py.test or nose. For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here: https://github.com/girving/numpy/tree/fixuserloops The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so. Geoffrey
On Tue, Dec 20, 2011 at 7:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
Hey, that's great! Thanks for getting this all together. Chuck
This is really excellent. I would like to take a stab at getting this pulled in to the code base --- and fixing the GIL issue --- if someone hasn't beat me to it. Travis -- Travis Oliphant (on a mobile) 512-826-7480 On Dec 20, 2011, at 9:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
Geoffrey _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
On Tue, Dec 20, 2011 at 6:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
Looks great. I've added some comments to the pull request for the fixuserloops branch, which is here: https://github.com/numpy/numpy/pull/175 I would advise anyone with an interest in the low-level aspects of how NumPy's handling of the GIL and multi-threading/concurrency should evolve to take a look. Prior to anything I contributed, NumPy hardcoded whether to release the GIL during ufuncs or not. I added a needs_api flag in a few places to indicate whether the inner loop functions call the CPython API or not. Note that for ABI compatibility reasons, this flag is not 100% correctly integrated throughout NumPy. What Geoffrey is proposing here conflicts with the way I imagined the flag would be used, but supporting both of our ways of calling the inner loop seems useful to me. Take a look at the pull request for more details. Cheers, Mark
Geoffrey _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
On Tue, Dec 20, 2011 at 9:10 PM, Mark Wiebe <mwwiebe@gmail.com> wrote:
On Tue, Dec 20, 2011 at 6:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
Looks great. I've added some comments to the pull request for the fixuserloops branch, which is here:
https://github.com/numpy/numpy/pull/175
I would advise anyone with an interest in the low-level aspects of how NumPy's handling of the GIL and multi-threading/concurrency should evolve to take a look. Prior to anything I contributed, NumPy hardcoded whether to release the GIL during ufuncs or not. I added a needs_api flag in a few
So releasing the GIL wasn't something the user could get at? (I'm curious if this is something that should be mentioned in the ufunc tutorial on the numpy docs.)
places to indicate whether the inner loop functions call the CPython API or not. Note that for ABI compatibility reasons, this flag is not 100% correctly integrated throughout NumPy.
Could you expand a little bit on this ABI compatibility issue?
What Geoffrey is proposing here conflicts with the way I imagined the flag would be used, but supporting both of our ways of calling the inner loop seems useful to me. Take a look at the pull request for more details.
Cheers, Mark
Geoffrey _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
On Tue, Dec 20, 2011 at 10:48 PM, Christopher Jordan-Squire <cjordan1@uw.edu
wrote:
On Tue, Dec 20, 2011 at 9:10 PM, Mark Wiebe <mwwiebe@gmail.com> wrote:
On Tue, Dec 20, 2011 at 6:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
Looks great. I've added some comments to the pull request for the fixuserloops branch, which is here:
https://github.com/numpy/numpy/pull/175
I would advise anyone with an interest in the low-level aspects of how NumPy's handling of the GIL and multi-threading/concurrency should evolve to take a look. Prior to anything I contributed, NumPy hardcoded whether to release the GIL during ufuncs or not. I added a needs_api flag in a few
So releasing the GIL wasn't something the user could get at? (I'm curious if this is something that should be mentioned in the ufunc tutorial on the numpy docs.)
User C/C++ code can release the GIL with some macros provided by NumPy, and it is accessible in ufuncs. The cases I'm talking about are where a custom data type, such as this rational type, supplies low level inner loop functions to do casting and various primitive calculations. These APIs don't in general provide ways to tell the NumPy outer loop code whether it's safe to release the GIL before calling the functions.
places to indicate whether the inner loop functions call the CPython API or
not. Note that for ABI compatibility reasons, this flag is not 100% correctly integrated throughout NumPy.
Could you expand a little bit on this ABI compatibility issue?
This boils down to the main reason that any significant changes to NumPy are currently unnecessarily difficult, that the ABI exposes the binary layout of key objects like ndarray and dtype. This means that when ABI compatibility is required, any changes have to be very careful not to disturb the binary layout of these objects. Changing something like how the needs_api flag works would change the nditer C API, which is part of the ABI as well. When the memory layouts are hidden from the ABI, it will be possible to make drastic changes to them while emulating the previous behavior in existing API calls, making it much easier to attempt changes that would improve performance or functionality. -Mark
What Geoffrey is proposing here conflicts with the way I imagined the
flag
would be used, but supporting both of our ways of calling the inner loop seems useful to me. Take a look at the pull request for more details.
Cheers, Mark
Geoffrey _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
Hi Geoffrey, On Tue, Dec 20, 2011 at 7:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
A few preliminary comments on the C code (since I can't comment directly on github) 1) The C++ style comments aren't portable. 2) The trailing comments would (IMHO) look better on the line above. 3) The inline keyword isn't portable, use NPY_INLINE instead. 4) We've mostly used the int foo(void) { } style of function definition. 5) And for if statements if (is_toohot) { change_seats(); } else if (is_toocold) { change_seats(); } else { eat_cereal(); } 6) Because Python assert disappears in release code, the tests need to use assert_(...) imported from numpy.testing Chuck
On Wed, Dec 21, 2011 at 3:56 AM, Charles R Harris <charlesr.harris@gmail.com> wrote:
Hi Geoffrey,
On Tue, Dec 20, 2011 at 7:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
A few preliminary comments on the C code (since I can't comment directly on github)
1) The C++ style comments aren't portable.
2) The trailing comments would (IMHO) look better on the line above.
3) The inline keyword isn't portable, use NPY_INLINE instead.
4) We've mostly used the
int foo(void) { }
style of function definition.
5) And for if statements
if (is_toohot) { change_seats(); } else if (is_toocold) { change_seats(); } else { eat_cereal(); }
6) Because Python assert disappears in release code, the tests need to use assert_(...) imported from numpy.testing
All fixed. Geoffrey
On Wed, Dec 21, 2011 at 1:37 PM, Geoffrey Irving <irving@naml.us> wrote:
On Wed, Dec 21, 2011 at 3:56 AM, Charles R Harris <charlesr.harris@gmail.com> wrote:
Hi Geoffrey,
On Tue, Dec 20, 2011 at 7:24 PM, Geoffrey Irving <irving@naml.us> wrote:
Hello,
As a followup to the prior thread on bugs in user defined types in numpy, I converted my rational number class from C++ to C and switched to 32 bits to remove the need for unportable 128 bit numbers. It should be usable as a fairly thorough test case for user defined types now. It does rather more than a minimal test case would need to do, but that isn't a problem unless you're concerned about code size. Let me know if any further changes are needed before it's suitable for inclusion in numpy as a test case. The repository is here:
https://github.com/girving/rational
The tests run under either py.test or nose.
For completeness, my branch fixing all but one of the bugs I found in numpy user defined types is here:
https://github.com/girving/numpy/tree/fixuserloops
The remaining bug is that numpy incorrectly releases the GIL during casts even though NPY_NEEDS_API is set. The resulting crash goes away if the line defining ACQUIRE_GIL is uncommented. With the necessary locks in place, all my tests pass with my branch of numpy. I haven't tracked this one down and fixed it yet, but it shouldn't be hard to do so.
A few preliminary comments on the C code (since I can't comment directly on github)
1) The C++ style comments aren't portable.
2) The trailing comments would (IMHO) look better on the line above.
3) The inline keyword isn't portable, use NPY_INLINE instead.
4) We've mostly used the
int foo(void) { }
style of function definition.
5) And for if statements
if (is_toohot) { change_seats(); } else if (is_toocold) { change_seats(); } else { eat_cereal(); }
6) Because Python assert disappears in release code, the tests need to use assert_(...) imported from numpy.testing
All fixed.
Mark, I'm ready to merge pull-175, is there any reason not to? Chuck
participants (5)
-
Charles R Harris -
Christopher Jordan-Squire -
Geoffrey Irving -
Mark Wiebe -
Travis Oliphant