Issue2244

New issue to separate this from related #2237.

The Jython implementation of a few math functions has been inaccurate in parts of their domain, and this has gone unnoticed by test_math.

In test.test_math, class MathTests tests each of the methods in math at various special values, such as sin(pi/2) and exp(1). The success criterion, applied in MathTests.ftest, is that the absolute difference from the expected value be at most 1e-5.

In addition, MathTests.test_testfile takes examples from cmath_testcases.txt (which test_cmath also uses), and using only those cases where the argument and the result are both real, once more uses ftest and a 1e-5 tolerance.

This is not a very tight criterion, and for several functions the cases provided, if there are any at all, do not explore interesting parts of the domain. In contrast, test.test_cmath finds a more interesting set of test cases and tolerates much smaller errors.

The number of positive and negative arguments with which each function is tested, and their range, is listed here:

acos      16 +ve 1e-323                   <= x <= 0.9999999999999999      
          18 -ve -1.0                     <= x <= -5e-324                 
acosh     16 +ve 1.0000000000000002       <= x <= 1.5653640340214026e+308 
asin      14 +ve 1e-323                   <= x <= 0.9999999999999999      
          16 -ve -1.0                     <= x <= -1e-323                 
asinh      4 +ve 3.38339e-318             <= x <= 1.6025136110019349e+308 
           4 -ve -1.2775271979042634e+308 <= x <= -5e-324                 
atan       2 +ve 1.2956339389525244e+308  <= x <= 1.440812624377215e+308  
           2 -ve -1.0631786461936417e+308 <= x <= -1.0516056964171069e+308
atanh     16 +ve 1e-323                   <= x <= 0.9999999999999999      
          16 -ve -0.9999999999999999      <= x <= -1e-323                 
cos        0 +ve None                     <= x <= None                    
           0 -ve None                     <= x <= None                    
cosh       0 +ve None                     <= x <= None                    
           0 -ve None                     <= x <= None                    
exp        0 +ve None                     <= x <= None                    
           1 -ve -745.0                   <= x <= -745.0                  
log       32 +ve 1e-323                   <= x <= 1.440860577601428e+308  
log10     32 +ve 1e-323                   <= x <= 1.440860577601428e+308  
sin        0 +ve None                     <= x <= None                    
           0 -ve None                     <= x <= None                    
sinh       0 +ve None                     <= x <= None                    
           0 -ve None                     <= x <= None                    
sqrt      30 +ve 1e-323                   <= x <= 1e+299                  
tan        0 +ve None                     <= x <= None                    
           0 -ve None                     <= x <= None                    
tanh       0 +ve None                     <= x <= None                    
           0 -ve None                     <= x <= None                    

The original approach may be satisfactory for CPython math because it is little more than a facade on the C math library, and must also tolerate platform variations. The java.lang.Math class underlying Jython's math makes strong guarantees about accuracy.

I propose to augment cmath_testcases.txt with real-valued cases to cover the domain of each of these functions, and apply a tighter acceptance criterion.

+1, this is an awesome investigation into these important details

On OS X 10.10.1 (Mavericks), I needed a higher tolerance for one of the cosh computations:

======================================================================
FAIL: testCosh (__main__.MathAccuracy)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jbaker/jythondev/jython27/dist/Lib/test/test_math.py", line 344, in testCosh
    self.ftest('cosh(2)-2*cosh(1)**2', math.cosh(2)-2*math.cosh(1)**2, -1) # Thanks to Lambert
  File "dist/Lib/test/test_math_jy.py", line 59, in ftest
    self.fail(message)
AssertionError: cosh(2)-2*cosh(1)**2 returned -1.0000000000000009, expected -1

----------------------------------------------------------------------
Ran 43 tests in 3.688s

FAILED (failures=1)
Traceback (most recent call last):
  File "dist/Lib/test/test_math_jy.py", line 75, in <module>
    test_main()
  File "dist/Lib/test/test_math_jy.py", line 68, in test_main
    test_support.run_unittest(
  File "/Users/jbaker/jythondev/jython27/dist/Lib/test/test_support.py", line 1274, in run_unittest
    _run_suite(suite)
  File "/Users/jbaker/jythondev/jython27/dist/Lib/test/test_support.py", line 1257, in _run_suite
    raise TestFailed(err)
test.test_support.TestFailed: Traceback (most recent call last):
  File "/Users/jbaker/jythondev/jython27/dist/Lib/test/test_math.py", line 344, in testCosh
    self.ftest('cosh(2)-2*cosh(1)**2', math.cosh(2)-2*math.cosh(1)**2, -1) # Thanks to Lambert
  File "dist/Lib/test/test_math_jy.py", line 59, in ftest
    self.fail(message)
AssertionError: cosh(2)-2*cosh(1)**2 returned -1.0000000000000009, expected -1

Likely this accuracy has been updated since release 21 (OS X), and before release 55 (Windows 8.1). However, checking the release notes (http://www.oracle.com/technetwork/java/javase/7u-relnotes-515228.html), I didn't see anything specific, likely because this change to cosh is too minor to be specifically addressed.

Using the latest version of each of our primary supported platforms, including all upgrades applied, here's what sys.platform reports:

OS X 10.10.1 - "java1.7.0_21"
Ubuntu 14.10 - "java1.7.0_60"
Windows 8.1 - "java1.7.0_55"

We may want to mark this as an expected failure in the regrtest, tied to the Java release (using sys.platform), but I don't think we should apply the below diff:

diff -r ea036792f304 Lib/test/test_math_jy.py
--- a/Lib/test/test_math_jy.py	Sun Jan 04 09:51:07 2015 -0700
+++ b/Lib/test/test_math_jy.py	Sun Jan 04 11:33:09 2015 -0700
@@ -48,7 +48,10 @@
     def ftest(self, name, value, expected):
         if expected != 0. :
             # Tolerate small deviation in proportion to expected
-            tol = Math.ulp(expected)
+            if name == 'cosh(2)-2*cosh(1)**2':
+                tol = Math.ulp(expected * 4)
+            else:
+                tol = Math.ulp(expected)
         else :
             # On zero, allow 2**-52. Maybe allow different slack based on name
             tol = Math.ulp(1.)

Something similar happened to me when I switched from our own implementation 0.5*(Math.exp(x)+Math.exp(-x)) to Math.cosh(x). I don't think this is because the Java library is worse, just slightly worse at that particular identity. In that expression, in theory, the tolerance needs to be 15*ulp(1), but I went for 5, provisionally.
https://hg.python.org/jython/rev/7402bd620d52

I hadn't actually checked up to that point whether there was a Math.cosh (and sinh and tanh). I just accepted that since we'd implemented it, we must have had to. Maybe in Java 1.4?

But there is now, and several others to save us doing things the hard way. The math module is now, much more than it was, a façade on java.lang.Math. Java tells us how accurate its Math is and I've implemented a corresponding tolerance in test_testfile.

This is an awesome set of changes. In particular, I like how it's now so much simpler (because of the use of standard Java Math) and cleaner. Well done!

Right, cosh, sinh, and tanh were only added since Java 5. It has certainly has been worked on more recently than Jython 2.1 days and Java 1.4, but I didn't dig any deeper than that.

I've reached the end of cmath re-work and these tests (as modified) pass the code. So consider testing strengthened. (Push shortly.)