'sympy/holonomic/tests/test_holonomic.py' fails on master

[iammosespaulr]

The Complete Logs

(sympy-dev-py35) mosespaul@eiphohch0aYa sympy2 % git checkout master
Already on 'master'
Your branch is up to date with 'upstream/master'.
(sympy-dev-py35) mosespaul@eiphohch0aYa sympy2 % python             
Python 3.5.5 | packaged by conda-forge | (default, Jul 23 2018, 23:45:11) 
[GCC 4.2.1 Compatible Apple LLVM 6.1.0 (clang-602.0.53)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from sympy import *
>>> test('sympy/holonomic/tests/test_holonomic.py')
====================================================================================== test process starts ======================================================================================
executable:         /Users/mosespaul/opt/anaconda3/envs/sympy-dev-py35/bin/python  (3.5.5-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
numpy:              1.18.2
random seed:        2125066
hash randomization: on (PYTHONHASHSEED=4270562452)

sympy/holonomic/tests/test_holonomic.py[27] .............E.....E.......                                                                                                                    [FAIL]

_________________________________________________________________________________________ slowest tests _________________________________________________________________________________________
test_integrate - Took 10.437 seconds
_________________________________________________________________________________________________________________________________________________________________________________________________
________________________________________________________________ sympy/holonomic/tests/test_holonomic.py:test_expr_to_holonomic _________________________________________________________________
Traceback (most recent call last):
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/tests/test_holonomic.py", line 489, in test_expr_to_holonomic
    p = expr_to_holonomic(besselj(2, x))
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 2390, in expr_to_holonomic
    sol = _convert_meijerint(func, x, initcond=False, domain=domain)
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 2820, in _convert_meijerint
    sol = fac_list[0] * coeff * from_meijerg(m, initcond=initcond, domain=domain)
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 2252, in from_meijerg
    r2 *= x * Dx - b[i]
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 288, in __sub__
    return self + (-1) * other
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 276, in __add__
    list_other = [((self.parent).base).from_sympy(sympify(other))]
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/polys/domains/old_polynomialring.py", line 251, in from_sympy
    rep[k] = self.dom.from_sympy(v)
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/polys/domains/pythonrationalfield.py", line 40, in from_sympy
    raise CoercionFailed("expected `Rational` object, got %s" % a)
sympy.polys.polyerrors.CoercionFailed: expected `Rational` object, got nan
_________________________________________________________________________________________________________________________________________________________________________________________________
____________________________________________________________________ sympy/holonomic/tests/test_holonomic.py:test_to_meijerg ____________________________________________________________________
Traceback (most recent call last):
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/tests/test_holonomic.py", line 718, in test_to_meijerg
    assert hyperexpand(expr_to_holonomic(besselj(2, x), lenics=3).to_meijerg()) == besselj(2, x)
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 2390, in expr_to_holonomic
    sol = _convert_meijerint(func, x, initcond=False, domain=domain)
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 2820, in _convert_meijerint
    sol = fac_list[0] * coeff * from_meijerg(m, initcond=initcond, domain=domain)
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 2252, in from_meijerg
    r2 *= x * Dx - b[i]
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 288, in __sub__
    return self + (-1) * other
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/holonomic/holonomic.py", line 276, in __add__
    list_other = [((self.parent).base).from_sympy(sympify(other))]
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/polys/domains/old_polynomialring.py", line 251, in from_sympy
    rep[k] = self.dom.from_sympy(v)
  File "/Users/mosespaul/Desktop/Coding/GSoC/sympy2/sympy/polys/domains/pythonrationalfield.py", line 40, in from_sympy
    raise CoercionFailed("expected `Rational` object, got %s" % a)
sympy.polys.polyerrors.CoercionFailed: expected `Rational` object, got nan

=================================================================== tests finished: 25 passed, 2 exceptions, in 45.79 seconds ===================================================================
DO *NOT* COMMIT!
False
>>> 

[jksuom]

It seems that different instances of Symbol may have the same hash value. That makes them indistinguishable in the cache. Therefore, comparisons of Symbols should use == instead of is here, and probably also elsewhere.

sympy/sympy/holonomic/holonomic.py

Lines 2809 to 2815 in 0b42f52

	t = b.as_base_exp()
	b = t[1] if t[0] is x else S.Zero
	r = s / b
	an = (i + r for i in func.args[0][0])
	ap = (i + r for i in func.args[0][1])
	bm = (i + r for i in func.args[1][0])
	bq = (i + r for i in func.args[1][1])

It looks like a version of besselj(2, x) with a slightly different x is cached earlier in the test series and then reused in test_expr_to_holonomic(). This is what happens:

> /home/jks/test/sympy/sympy/holonomic/holonomic.py(2812)_shift()
-> b = t[1] if t[0] is x else S.Zero
(Pdb) p t[0], x
(x, x)
(Pdb) p t[0] is x
False
(Pdb) p t[0] == x
True
(Pdb) p hash(t[0]) == hash(x)
True

The test t[0] is x fails making b == 0 and then i + r == nan in bm and bq.

[iammosespaulr]

Yeah, the tests ran individually but running the entire file seems to fail. How come this doesn't get caught on travis ? something to do with the Splitting ?

[oscarbenjamin]

If the failure is cache dependent then it can depend on the order in which tests are run.

[iammosespaulr]

It seems that different instances of Symbol may have the same hash value. That makes them indistinguishable in the cache.

Isn't this a bomb waiting to explode 😄, Ideally they shouldn't have the same value unless they are the thing right ? ( I haven't gone through the caching part of the sympy codebase yet )

[iammosespaulr]

If the failure is cache dependent then it can depend on the order in which tests are run.

So fixing the cache system should prevent these right ? cause the code seems alright on it's own.

> (Pdb) p t[0] is x
> False
> (Pdb) p t[0] == x
> True

This looks like compromising in order to escape potential caching error

[oscarbenjamin]

There is no problem with the cache here. The code that is using is should be changed to use == because using is only happens to work some of the time due to the cache. Without the cache this code would fail. If it is fixed then it will always work with or without the cache.

[iammosespaulr]

There is no problem with the cache here. The code that is using is should be changed to use == because using is only happens to work some of the time due to the cache. Without the cache this code would fail. If it is fixed then it will always work with or without the cache.

Ahhh okay, I’ll push a fix for that 😁