Skip to content
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

Numpy 1.17.0 issues #6715

Closed
jamesjer opened this issue Aug 14, 2019 · 4 comments · Fixed by #6749
Closed

Numpy 1.17.0 issues #6715

jamesjer opened this issue Aug 14, 2019 · 4 comments · Fixed by #6749

Comments

@jamesjer
Copy link

I maintain the Theano package for the Fedora Linux distribution. We recently built all packages for the upcoming Fedora 31, and the Theano package, version 1.0.4, failed its tests. There was 1 ERROR and 4 FAILs. I tracked the issue down to the introduction of numpy 1.17.0 in Fedora. See https://bugzilla.redhat.com/show_bug.cgi?id=1737011 for more information. Here is what I have figured out so far.

The ERROR is because of changes to numpy's ceil, floor, and trunc ufuncs. To fix it, class _tensor_py_operators in theano/tensor/var.py needs __ceil__, __floor__, and __trunc__ methods that do exactly what the current ceil, floor, and trunc methods do.

The FAIL in theano/gof/tests/test_compute_test_value.py is because there is now one extra frame on the stack. Line 289 of theano/gof/tests/test_compute_test_value.py reads:

                frame_info = traceback.extract_tb(tb)[-5]

but the -5 has to be a -6 for numpy 1.17.0.

The FAIL in theano/tensor/tests/test_basic.py is because the behavior of numpy's clip function changed. With numpy 1.16.4:

>>> np.clip(3, 5, 2)
5

but with numpy 1.17.0:

>>> np.clip(3, 5, 2)
2

I think the order of comparing x to min and max in Clip.impl (theano/scalar/basic.py) has to be reversed for numpy 1.17.0, if theano's clip is to behave like numpy's, as this test indicates.

I do not know what is causing the last two FAILs. Two random number tests used to raise ValueError and now do not raise any error at all.

@nouiz
Copy link
Member

nouiz commented Sep 3, 2019

Thanks for bringing this to our attention.In the past, if numpy semantic was changing (as is the case here), we where following it.
But as Theano is very lightly maintained now, this would cause old code to behave differently depending of the NumPy version.
We do not want that now as people do not maintain well old codes expecially for a not-well-maintained software.
So I see 2 options:

  • Just disable those functions. But this would break old codes, so this is not great.
  • Fix our test to make it pass with newer and older numpy version, but keep the old semantic. I would also print a warning that the function use the old numpy semantic in those cases.

What do you think of that? Do you have time to work on a PR that does the second option?

@jamesjer
Copy link
Author

I'm not sure I have the knowledge necessary for that second option. I can give you a PR that fixes the ERROR, but I am not at all certain what should be done about the 4 FAILs.

@nouiz
Copy link
Member

nouiz commented Sep 17, 2019

Can you show all the errors?

@jamesjer
Copy link
Author

See #6721 for a resolution to the ERROR. Here are the 4 FAILs:

======================================================================
FAIL: test_scan_err1 (theano.gof.tests.test_compute_test_value.TestComputeTestValue)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/Theano-rel-1.0.4/theano/gof/tests/test_compute_test_value.py", line 283, in test_scan_err1
    n_steps=k)
ValueError: shapes (5,3) and (5,3) not aligned: 3 (dim 1) != 5 (dim 0)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/builddir/build/BUILD/Theano-rel-1.0.4/theano/gof/tests/test_compute_test_value.py", line 292, in test_scan_err1
    assert os.path.split(frame_info[0])[1] == expected, frame_info
AssertionError: <FrameSummary file /builddir/build/BUILD/Theano-rel-1.0.4/theano/tensor/basic.py, line 6105 in dot>

======================================================================
FAIL: test_good (theano.tensor.tests.test_basic.ClipTester)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/Theano-rel-1.0.4/theano/tensor/tests/test_basic.py", line 425, in test_good
    np.allclose(variable, expected)))
AssertionError: Test Elemwise{clip,no_inplace}::correct7: Output 0 gave the wrong value. With inputs [array([[-1.70586872,  2.46664369, -0.52189608,  4.84714482,  1.9208813 ],
       [ 1.28983643, -2.38914901, -1.67933224,  2.10808673,  3.58489085],
       [ 4.45900732, -3.70413992, -3.7683781 ,  1.87495988, -2.58993616],
       [-2.6853165 ,  2.19925062,  2.69281351,  1.39211517,  1.29298677],
       [ 4.43245431,  3.63622689, -2.58812035, -3.32239248,  0.24149971]]), array(1.), array(-1.)], expected [[-1. -1. -1. -1. -1.]
 [-1. -1. -1. -1. -1.]
 [-1. -1. -1. -1. -1.]
 [-1. -1. -1. -1. -1.]
 [-1. -1. -1. -1. -1.]] (dtype float64), got [[ 1. -1.  1. -1. -1.]
 [-1.  1.  1. -1. -1.]
 [-1.  1.  1. -1.  1.]
 [ 1. -1. -1. -1. -1.]
 [-1. -1.  1.  1.  1.]] (dtype float64). eps=0.000000 np.allclose returns False False

======================================================================
FAIL: test_vector_arguments (theano.tensor.tests.test_raw_random.T_random_function)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/Theano-rel-1.0.4/theano/tensor/tests/test_raw_random.py", line 681, in test_vector_arguments
    self.assertRaises(ValueError, fc, post1c, [-4., -2], [-1, 0], [1])
AssertionError: ValueError not raised by <theano.compile.function_module.Function object at 0x7fcab1fdf890>

======================================================================
FAIL: test_vector_arguments (theano.tensor.tests.test_shared_randomstreams.T_SharedRandomStreams)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/builddir/build/BUILD/Theano-rel-1.0.4/theano/tensor/tests/test_shared_randomstreams.py", line 469, in test_vector_arguments
    self.assertRaises(ValueError, fc, [-4., -2], [-1, 0], [1])
AssertionError: ValueError not raised by <theano.compile.function_module.Function object at 0x7fcab178de90>

----------------------------------------------------------------------

rebecca-palmer added a commit to rebecca-palmer/Theano that referenced this issue Apr 2, 2020
twiecki referenced this issue in aesara-devs/aesara Apr 10, 2020
* Fix subtensor numpy warning

* Fix simple typo: varible->size -> variable->size

Closes #6734

* Counteract hypot redefinition for old Python versions

* fix doc warning

* Update extending_theano.txt

* OrderedDict cannot be imported from collections.abc, it is not abstract

See Theano/Theano#6664 (comment)

* Move more collections.abc imports to theano.compat

* Use math.gcd instead of fractions.gcd when possible

fractions.gcd() function has been removed from Python 3.9, it was
deprecated since Python 3.5.

https://docs.python.org/3.9/whatsnew/3.9.html#removed

* DOC: Allow building with Sphinx >= 2.0

* MAINT: Avoid SyntaxWarnings on import in Python 3.8

* flake8 code style fixes

* DOC: Retain compatibility with older Sphinx

* MAINT: Be compatible with numpy 1.17 and scipy 1.3

Fixes #6715.  Includes and supersedes #6721.

* flake8 code style fixes

* Remove doctest in python 3.4

* Do not install sphinx_rtd_theme for python 3.4 as the installation doesn't work anymore.

* fixing typo in GammaIncC Op

* re-add min > max clip test with fixed reference

* DOC: min > max clip may not match numpy

* updating travis build for Python 3

* order of stages

* conda activate

* installing conda properly

* conda install on travis

* conda activate issues on travis

* conda activate issues on travis

* doctest needs older version of numpy

* doctest

* reverting setup.cfg

* conda version conflicts

* need old sphinx version

* sphinx version

* don't bother building docs

* moved nosetester import

* also moving known failures plugin

Co-authored-by: Adrian Seyboldt <adrian.seyboldt@gmail.com>
Co-authored-by: Frédéric Bastien <frederic.bastien@gmail.com>
Co-authored-by: Tim Gates <tim.gates@iress.com>
Co-authored-by: Marcel Bargull <marcel.bargull@udo.edu>
Co-authored-by: Arnaud Bergeron <abergeron@gmail.com>
Co-authored-by: Miro Hrončok <miro@hroncok.cz>
Co-authored-by: Rebecca N. Palmer <rebecca_palmer@zoho.com>
Co-authored-by: Frederic Bastien <fbastien@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants