Re: Fundamental scipy testing changes
The discussion below relates to porting scipy to numarray, starting with some small changes to scipy_test.testing. The discussion began as private e-mail but has crossed over to scipy-dev at Pearu's suggestion. On Tue, 2004-12-07 at 16:16, Pearu Peterson wrote:
Hi Todd,
On 7 Dec 2004, Todd Miller wrote:
I took a closer look at how my scipy_base changes affect scipy.test() and found a couple things in my scipy_base changes that needed fixing. Sorry I didn't think of this sooner. These fixes are committed now on the numarray branch in CVS. I also honed in on what I think should be the starting point for discussion: the changes I made to scipy_test/testing.py.
Note that assert_equal, assert_almost_equal, assert_approx_equal were not meant to be used with array arguments (I didn't implement them but its obvious from reading the code).
Thanks for pointing this out... I noticed the array versions only peripherally and didn't understand the distinction.
For checking the equality of array arguments assert_array_equal or assert_array_almost_equal should be used. If some scipy test suite uses assert_equal, etc with array arguments then I think this is a bug of this particular test suite, not of testing.py. So, using scipy_base.all in assert_equal, etc is not necessary (unless we want to drop assert_array_* functions).
Understood. Are we agreed that it is appropriate to use all() in the assert_array_* functions?
In order to do the numarray scipy_base changes and pass scipy_base.test(), I modified scipy_test/testing.py in ways which I assert are good. Mostly, I used all() in contexts where an array result was being used as a truth value. In those contexts, the array __nonzero__() function is executed. NumArray.__nonzero__() raises an exception causing many tests to fail that should succeed. Using all() reduces the "truth array" to a single scalar value, so NumArray.__nonzero__() is never called.
Where I think early agreement needs to happen is that my testing.py changes are good even though they expose a handful of latent scipy bugs or unit test bugs because Numeric's __nonzero__() has the meaning of any() and not all(). So, for say an equality test, if any of the values were equal, the test would succeed even if most of the arrays' values were *not* equal. Switching to all() as I am advocating exposes hidden problems like these.
At the moment I have no comments on this as I haven't tried the numarray branch of scipy yet.
I think we should square away the scipy_test.testing changes before anyone messes with the numarray branch.
Btw, your discussion about the changes to scipy sounds reasonable. However, why do you say that using alltrue is an error? I thought all(x) is equivalent to alltrue(ravel(x)) and in assert_array_* functions the arguments to alltrue are already ravel'ed.
I looked at my diffs more carefully and see that you're right, the alltrues are not bugs because they're only fed 1D arrays. I have not reexamined all my scipy_base changes for instances of alltrue.
If my testing.py changes are agreed upon and the exposed bugs are either fixed or acknowledged as known, we have a better basis for examining the rest of the numarray changes to scipy_base.
I attached 3 files for you to look at:
1. testing.diffs (changes to -r HEAD of scipy_test/testing.py)
2. results.HEAD (scipy.test() results against the HEAD of CVS using Numeric) I got 4 failures and can't see the nightly scoreboard yet so I don't know if these are expected.
3. results.testing.changes (scipy.test() results with testing.diffs applied) I got 11 failures (including the original 4) most of which I believe are either bugs or unit test bugs.
The failures, where you see almost equal arrays, may occur when using Fortran blas;
My scipy.test() does complain about not finding clapack so maybe I am using a Fortran blas too.
these errors should be fixed by using proper decimal argument in assert_array_almost_equal call of the corresponding tests (I thought I have fixed these errors in scipy HEAD already).
As you said I think, the (incorrect) changes to assert_equal exposed (incorrect) uses of assert_equal in array contexts.
Other failures need more attention.
If any of you would rather not be included in my future numarray mailings, or you have suggestions for a better venue for them, please let me know.
I suggest using scipy-dev.
Thanks, Pearu
I re-attached the attachments in case anyone on scipy-dev wants to look as well; there's nothing new there. Thanks for looking it over, Todd
On Tue, 7 Dec 2004, Todd Miller wrote:
Note that assert_equal, assert_almost_equal, assert_approx_equal were not meant to be used with array arguments (I didn't implement them but its obvious from reading the code).
Thanks for pointing this out... I noticed the array versions only peripherally and didn't understand the distinction.
May be we need to review assert_equal, etc so that they will handle array inputs similar to assert_array_equal but on Python objects they will not use unnecessary scipy_base.all. And then define assert_array_equal = assert_equal assert_array_almost_equal = assert_almost_equal in testing.py for backward compability and declare their use as depreciated.
For checking the equality of array arguments assert_array_equal or assert_array_almost_equal should be used. If some scipy test suite uses assert_equal, etc with array arguments then I think this is a bug of this particular test suite, not of testing.py. So, using scipy_base.all in assert_equal, etc is not necessary (unless we want to drop assert_array_* functions).
Understood. Are we agreed that it is appropriate to use all() in the assert_array_* functions?
Yes. But be careful when replacing `if obj:` with `if all(obj):` in other parts of scipy as it may also mean `if any(obj):` or `if obj is not None:`, in fact, I think these are being assumed in most cases. And if not, then it should be a bug. I agree that the usage of `if obj:` is a bug and should be fixed either to `if any(obj):` or `if obj is not None:` or rarely `if all(obj):`. Pearu
On Wed, 2004-12-08 at 06:53, Pearu Peterson wrote:
On Tue, 7 Dec 2004, Todd Miller wrote:
Note that assert_equal, assert_almost_equal, assert_approx_equal were not meant to be used with array arguments (I didn't implement them but its obvious from reading the code).
Thanks for pointing this out... I noticed the array versions only peripherally and didn't understand the distinction.
May be we need to review assert_equal, etc so that they will handle array inputs similar to assert_array_equal but on Python objects they will not use unnecessary scipy_base.all. And then define
assert_array_equal = assert_equal assert_array_almost_equal = assert_almost_equal
in testing.py for backward compability and declare their use as depreciated.
This sounds like a good interface simplification. I looked at doing the deprecation (sticking in warnings) but was quickly intimidated by the array equality functions. I think as an alternative to deprecation, we should consider having assert_equal delegate to assert_array_equal for arrays. That way, arrays are handled as they have been, but future testers won't have to distinguish between array and non-array contexts.
For checking the equality of array arguments assert_array_equal or assert_array_almost_equal should be used. If some scipy test suite uses assert_equal, etc with array arguments then I think this is a bug of this particular test suite, not of testing.py. So, using scipy_base.all in assert_equal, etc is not necessary (unless we want to drop assert_array_* functions).
Understood. Are we agreed that it is appropriate to use all() in the assert_array_* functions?
Yes.
But be careful when replacing `if obj:` with `if all(obj):` in other parts of scipy as it may also mean `if any(obj):` or `if obj is not None:`, in fact, I think these are being assumed in most cases. And if not, then it should be a bug.
Sounds good.
I agree that the usage of `if obj:` is a bug and should be fixed either to `if any(obj):` or `if obj is not None:` or rarely `if all(obj):`.
Something else worth explicitly mentioning is array comparisons and logical expressions, something like "if A<B:". For those, it's important to say "if all(A<B):" or numarray will raise an exception and Numeric will really mean "if any(A<B):". It might also be worth pointing out that "if A<B and C<D:" should be converted to "if any(A<B) and any(C<D):" and not "if any(A<B and C<D):". Thanks again for looking this over. Regards, Todd
A week or so ago we were discussing the set of changes to scipy's testing framework needed to add support for numarray. The changes I'm talking about here should be made to the main trunk of scipy CVS. After further review, I now think the array versions of the assert family of functions (e.g. assert_array_equal) are correct with respect to truth value testing. So, the only change I think is needed is the addition of "delegation code" so that calls to assert_equal, etc. defer to assert_array_equal, etc. when passed array parameters. Here's the patch: Index: testing.py =================================================================== RCS file: /home/cvsroot/world/scipy_core/scipy_test/testing.py,v retrieving revision 1.49 retrieving revision 1.49.2.3 diff -c -r1.49 -r1.49.2.3 *** testing.py 1 Dec 2004 07:08:51 -0000 1.49 --- testing.py 17 Dec 2004 18:20:28 -0000 1.49.2.3 *************** *** 634,639 **** --- 633,640 ---- """ Raise an assertion if two items are not equal. I think this should be part of unittest.py """ + if isinstance(actual, ArrayType): + return assert_array_equal(actual, desired, err_msg) msg = '\nItems are not equal:\n' + err_msg try: if ( verbose and len(repr(desired)) < 100 and len(repr(actual)) ): *************** *** 651,656 **** --- 652,659 ---- """ Raise an assertion if two items are not equal. I think this should be part of unittest.py """ + if isinstance(actual, ArrayType): + return assert_array_almost_equal(actual, desired, decimal, err_msg) msg = '\nItems are not equal:\n' + err_msg try: if ( verbose and len(repr(desired)) < 100 and len(repr(actual)) ): These changes are necessary for adding numarray support to scipy because without them there are 12 testing failures (in scipy_base.test()). The failures are all related to numarray's "outlawed" __nonzero__() and testers apparently calling the wrong assertion function. With these changes, 7 of the numarray failures go away and 5 identical failures remain for both numarray and Numeric. I'm arguing that the 5 remaining failures are either real problems or testing bugs which were masked by testers calling the wrong assert functions and by Numeric's sometrue() definition of __nonzero__(). Here are the test failures I see: ====================================================================== FAIL: check_basic (scipy_base.function_base.test_function_base.test_amax) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jmiller/work/lib/python2.4/site-packages/scipy_base/tests/test_function_base.py", line 72, in check_basic assert_equal(amax(b),[8.0,10.0,9.0]) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 638, in assert_equal return assert_array_equal(actual, desired, err_msg) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 721, in assert_array_equal assert cond,\ AssertionError: Arrays are not equal (mismatch 66.6666666667%): Array 1: [ 9. 10. 8.] Array 2: [ 8. 10. 9.] ====================================================================== FAIL: check_basic (scipy_base.function_base.test_function_base.test_amin) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jmiller/work/lib/python2.4/site-packages/scipy_base/tests/test_function_base.py", line 82, in check_basic assert_equal(amin(b),[3.0,3.0,2.0]) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 638, in assert_equal return assert_array_equal(actual, desired, err_msg) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 721, in assert_array_equal assert cond,\ AssertionError: Arrays are not equal (mismatch 33.3333333333%): Array 1: [ 3. 4. 2.] Array 2: [ 3. 3. 2.] ====================================================================== FAIL: check_arange (scipy.special.basic.test_basic.test_arange) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jmiller/work/lib/python2.4/site-packages/scipy/special/tests/test_basic.py", line 495, in check_arange assert_equal(numstring,array([0.,0.1,0.2,0.3, File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 638, in assert_equal return assert_array_equal(actual, desired, err_msg) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 721, in assert_array_equal assert cond,\ AssertionError: Arrays are not equal (mismatch 30.4347826087%): Array 1: [ 0. 0.1 0.2 0.3 0.4 0.5 ... Array 2: [ 0. 0.1 0.2 0.3 0.4 0.5 ... ====================================================================== FAIL: check_genlaguerre (scipy.special.basic.test_basic.test_laguerre) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jmiller/work/lib/python2.4/site-packages/scipy/special/tests/test_basic.py", line 1573, in check_genlaguerre assert_equal(lag2.c,array([1,-2*(k+2),(k+1.)*(k+2.)])/2.0) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 638, in assert_equal return assert_array_equal(actual, desired, err_msg) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 721, in assert_array_equal assert cond,\ AssertionError: Arrays are not equal (mismatch 66.6666666667%): Array 1: [ 0.5 -2.5842644468705798 2.0470791422443622] Array 2: [ 0.5 -2.5842644468705802 2.047079142244363 ] ====================================================================== FAIL: check_legendre (scipy.special.basic.test_basic.test_legendre) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/jmiller/work/lib/python2.4/site-packages/scipy/special/tests/test_basic.py", line 1590, in check_legendre assert_equal(leg3.c,array([5,0,-3,0])/2.0) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 638, in assert_equal return assert_array_equal(actual, desired, err_msg) File "/home/jmiller/work/lib/python2.4/site-packages/scipy_test/testing.py", line 721, in assert_array_equal assert cond,\ AssertionError: Arrays are not equal (mismatch 75.0%): Array 1: [ 2.5000000000000000e+00 -8.3266726846886741e-16 -1.4999999999999991e+00 1.1340467527089692e-17] Array 2: [ 2.5 0. -1.5 0. ] ---------------------------------------------------------------------- Ran 690 tests in 3.949s FAILED (failures=5) In each case you can see that assert_array_equal has been called from the new delegation code in assert_equal. At this point, I guess I have two questions: 1. Is this patch acceptable for the main trunk now? 2. If so, who should fix the test failures? Regards, Todd
On 22 Dec 2004, Todd Miller wrote:
A week or so ago we were discussing the set of changes to scipy's testing framework needed to add support for numarray. The changes I'm talking about here should be made to the main trunk of scipy CVS.
After further review, I now think the array versions of the assert family of functions (e.g. assert_array_equal) are correct with respect to truth value testing. So, the only change I think is needed is the addition of "delegation code" so that calls to assert_equal, etc. defer to assert_array_equal, etc. when passed array parameters. Here's the patch:
I have applied the patch with an addition of calling array versions of assert_* functions whenever either of arguments is an array.
These changes are necessary for adding numarray support to scipy because without them there are 12 testing failures (in scipy_base.test()). The failures are all related to numarray's "outlawed" __nonzero__() and testers apparently calling the wrong assertion function. With these changes, 7 of the numarray failures go away and 5 identical failures remain for both numarray and Numeric. I'm arguing that the 5 remaining failures are either real problems or testing bugs which were masked by testers calling the wrong assert functions and by Numeric's sometrue() definition of __nonzero__().
I agree. These 5 failures are now fixed in the main trunk.
In each case you can see that assert_array_equal has been called from the new delegation code in assert_equal.
At this point, I guess I have two questions:
1. Is this patch acceptable for the main trunk now?
Looks ok to me. The patch is applied.
2. If so, who should fix the test failures?
Anyone is welcome to fix the failures. The failures should be now fixed. Thanks, Pearu
Now that there will soon be some visible sign of numarray integration efforts, it seems to me that having some sort of page indicating the current state of integration would be helpful. That is, it would outline the general plan for the effort, indicate what's been done and is currently available for use with numarray, what being worked on and the general priorities. Users can use to see if the functionality they wish to use with numarray is already available (i.e., which libraries) or where the unfinished work sits in the queue. It can be used for developers to indicate where help would be useful if others want to contribute. Any objection to having such a page? Perry
I agree, Perry. Would a wiki page (folder) on scipy.org do the trick, or do you need something less dynamic? Please advise I'll be glad to set up whatever you need, or, if you don't already, I can make sure you have the appropriate permissions (and Todd, and whoever else wants them). Travis Perry Greenfield wrote:
Now that there will soon be some visible sign of numarray integration efforts, it seems to me that having some sort of page indicating the current state of integration would be helpful. That is, it would outline the general plan for the effort, indicate what's been done and is currently available for use with numarray, what being worked on and the general priorities. Users can use to see if the functionality they wish to use with numarray is already available (i.e., which libraries) or where the unfinished work sits in the queue. It can be used for developers to indicate where help would be useful if others want to contribute. Any objection to having such a page?
Perry
A wiki page would be fine. We can have various links to it when enough is ready (e.g., the "A discussion of scipy and numarray" which itself ought to be updated when some end user functionality is available for numarray and scipy (but for now I don't think it is worth bothering about, we'll be there soon enough)) Perry On Jan 11, 2005, at 12:15 PM, Travis N. Vaught wrote:
I agree, Perry.
Would a wiki page (folder) on scipy.org do the trick, or do you need something less dynamic?
Please advise I'll be glad to set up whatever you need, or, if you don't already, I can make sure you have the appropriate permissions (and Todd, and whoever else wants them).
Travis
Perry Greenfield wrote:
Now that there will soon be some visible sign of numarray integration efforts, it seems to me that having some sort of page indicating the current state of integration would be helpful. That is, it would outline the general plan for the effort, indicate what's been done and is currently available for use with numarray, what being worked on and the general priorities. Users can use to see if the functionality they wish to use with numarray is already available (i.e., which libraries) or where the unfinished work sits in the queue. It can be used for developers to indicate where help would be useful if others want to contribute. Any objection to having such a page?
Perry
_______________________________________________ Scipy-dev mailing list Scipy-dev@scipy.net http://www.scipy.net/mailman/listinfo/scipy-dev
participants (5)
-
Pearu Peterson -
Pearu Peterson -
Perry Greenfield -
Todd Miller -
Travis N. Vaught