sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.22k stars 424 forks source link

Upgrade SymPy to 1.10 and update doctests #33398

Closed mkoeppe closed 2 years ago

mkoeppe commented 2 years ago

SymPy 1.10 was released on 2022-03-06. It still supports Python 3.7, as needed for Sage 9.6.

https://github.com/sympy/sympy/wiki/release-notes-for-1.10

Some doctests needs updating.

From https://github.com/sympy/sympy/runs/5276327276?check_suite_focus=true

sage -t --random-seed=141799432516026950115879763089480345171 src/sage/calculus/calculus.py
**********************************************************************
File "src/sage/calculus/calculus.py", line 1642, in sage.calculus.calculus.laplace
Failed example:
    a, cond
Expected:
    (-oo, True)
Got:
    (0, True)
**********************************************************************

This appears to be a deliberate change in sympy. We update the doctest.

Some doctests need updating because of changes to expression canonicalization in sympy:

sage -t --random-seed=141799432516026950115879763089480345171 src/sage/manifolds/continuous_map.py
**********************************************************************
File "src/sage/manifolds/continuous_map.py", line 1359, in sage.manifolds.continuous_map.ContinuousMap.coord_functions
Failed example:
    Phi.coord_functions(c_UV, c_xyz)
Expected:
    Coordinate functions (-U**2/4 + V**2/4, -(U + V)/(U - V), V)
     on the Chart (M, (U, V))
Got:
    Coordinate functions (-U**2/4 + V**2/4, (-U - V)/(U - V), V) on the Chart (M, (U, V))

We update these doctests. They will fail now with older versions of sympy. However, we do not adjust the version bounds in build/pkgs/sympy/install-requires.txt.

CC: @egourgoulhon @oscarbenjamin

Component: symbolics

Author: Matthias Koeppe

Branch/Commit: 6b7c3a2

Reviewer: Eric Gourgoulhon

Issue created by migration from https://trac.sagemath.org/ticket/33398

mkoeppe commented 2 years ago
comment:1
sage -t --random-seed=141799432516026950115879763089480345171 src/sage/symbolic/expression_conversions.py
**********************************************************************
File "src/sage/symbolic/expression_conversions.py", line 870, in sage.symbolic.expression_conversions.SympyConverter.derivative
Failed example:
    df_sympy == f_sympy.diff(x, 2, y, 1)
Exception raised:
    Traceback (most recent call last):
      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.symbolic.expression_conversions.SympyConverter.derivative[5]>", line 1, in <module>
        df_sympy == f_sympy.diff(x, Integer(2), y, Integer(1))
      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sympy/core/expr.py", line 3533, in diff
        return _derivative_dispatch(self, *symbols, **assumptions)
      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sympy/core/function.py", line 1920, in _derivative_dispatch
        return Derivative(expr, *variables, **kwargs)
      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sympy/core/function.py", line 1345, in __new__
        raise ValueError(filldedent('''
    ValueError: 
    Can't calculate derivative wrt 2.
**********************************************************************

For this failure in src/sage/symbolic/expression_conversions.py, I have opened https://github.com/sympy/sympy/issues/23144

mkoeppe commented 2 years ago
comment:2

A few more that are only seen with sage -t --long:

sage -t --long --random-seed=247003911346858216390278955506857279024 src/sage/manifolds/differentiable/tensorfield.py
**********************************************************************
File "src/sage/manifolds/differentiable/tensorfield.py", line 334, in sage.manifolds.differentiable.tensorfield.TensorField
Failed example:
    f.display()  # long time
Expected:
    t(a,b): S^2 → ℝ
    on U: (x, y) ↦ -2*x*y - 3*x - y**2
    on V: (u, v) ↦ -(3*u**3 + 3*u*v**2 + 2*u*v + v**2)/(u**4 + 2*u**2*v**2 + v**4)
Got:
    t(a,b): S^2 → ℝ
    on U: (x, y) ↦ -2*x*y - 3*x - y**2
    on V: (u, v) ↦ (-3*u**3 - 3*u*v**2 - 2*u*v - v**2)/(u**4 + 2*u**2*v**2 + v**4)
**********************************************************************
File "src/sage/manifolds/differentiable/tensorfield.py", line 343, in sage.manifolds.differentiable.tensorfield.TensorField
Failed example:
    s.display()  # long time
Expected:
    t(a,b): U → ℝ
       (x, y) ↦ -2*x*y - 3*x - y**2
    on W: (u, v) ↦ -(3*u**3 + 3*u*v**2 + 2*u*v + v**2)/(u**4 + 2*u**2*v**2 + v**4)
Got:
    t(a,b): U → ℝ
       (x, y) ↦ -2*x*y - 3*x - y**2
    on W: (u, v) ↦ (-3*u**3 - 3*u*v**2 - 2*u*v - v**2)/(u**4 + 2*u**2*v**2 + v**4)
**********************************************************************
File "src/sage/manifolds/differentiable/tensorfield.py", line 348, in sage.manifolds.differentiable.tensorfield.TensorField
Failed example:
    s.display()  # long time
Expected:
    t(a,b): W → ℝ
       (x, y) ↦ -2*x*y - 3*x - y**2
       (u, v) ↦ -(3*u**3 + 3*u*v**2 + 2*u*v + v**2)/(u**4 + 2*u**2*v**2 + v**4)
Got:
    t(a,b): W → ℝ
       (x, y) ↦ -2*x*y - 3*x - y**2
       (u, v) ↦ (-3*u**3 - 3*u*v**2 - 2*u*v - v**2)/(u**4 + 2*u**2*v**2 + v**4)
**********************************************************************
File "src/sage/manifolds/differentiable/tensorfield.py", line 357, in sage.manifolds.differentiable.tensorfield.TensorField
Failed example:
    s.display()  # long time
Expected:
    t(a,b): V → ℝ
       (u, v) ↦ -(3*u**3 + 3*u*v**2 + 2*u*v + v**2)/(u**4 + 2*u**2*v**2 + v**4)
    on W: (x, y) ↦ -2*x*y - 3*x - y**2
Got:
    t(a,b): V → ℝ
       (u, v) ↦ (-3*u**3 - 3*u*v**2 - 2*u*v - v**2)/(u**4 + 2*u**2*v**2 + v**4)
    on W: (x, y) ↦ -2*x*y - 3*x - y**2
**********************************************************************
1 item had failures:
   4 of  82 in sage.manifolds.differentiable.tensorfield.TensorField
    [1059 tests, 4 failures, 716.84 s]
egourgoulhon commented 2 years ago
comment:3

Thanks for the report. What should be the action here? Shall we wait for the release of SymPy 1.10 and its integration as a Sage standard package? Otherwise, there will be doctest failures with SymPy 1.9 --- the current version in Sage.

mkoeppe commented 2 years ago
comment:4

Replying to @mkoeppe:

For the failure in src/sage/symbolic/expression_conversions.py, I have opened https://github.com/sympy/sympy/issues/23144

Fixed now in master and the 1.10 branch by https://github.com/sympy/sympy/pull/23162, https://github.com/sympy/sympy/pull/23166

mkoeppe commented 2 years ago
comment:5

Replying to @egourgoulhon:

What should be the action here? Shall we wait for the release of SymPy 1.10 and its integration as a Sage standard package?

Yes, I think so

mkoeppe commented 2 years ago

Replying to @mkoeppe:

From https://github.com/sympy/sympy/runs/5276327276?check_suite_focus=true

sage -t --random-seed=141799432516026950115879763089480345171 src/sage/calculus/calculus.py
**********************************************************************
File "src/sage/calculus/calculus.py", line 1642, in sage.calculus.calculus.laplace
Failed example:
    a, cond
Expected:
    (-oo, True)
Got:
    (0, True)
**********************************************************************

This change to laplace_transform seems to be a deliberate change in sympy, not a regression.

mkoeppe commented 2 years ago

Branch: u/mkoeppe/update_doctests_for_compatibility_with_sympy_1_10

mkoeppe commented 2 years ago

New commits:

cd66605build/pkgs/sympy: Update to 1.10rc3
c49eff3src/sage/calculus/calculus.py: Update laplace doctest for sympy 1.10
mkoeppe commented 2 years ago

Author: Matthias Koeppe

mkoeppe commented 2 years ago

Commit: c49eff3

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from c49eff3 to 609dd9d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

609dd9dsage.manifolds: Update doctests for SymPy 1.10
mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,9 @@
+SymPy 1.10 is in the release candidate stage; it still supports Python 3.7.
+
+Some doctests needs updating.
+
+​https://github.com/sympy/sympy/wiki/release-notes-for-1.10
+
 From https://github.com/sympy/sympy/runs/5276327276?check_suite_focus=true
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 609dd9d to 6b7c3a2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

6b7c3a2build/pkgs/sympy: Update to 1.10
mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,8 +1,8 @@
-SymPy 1.10 is in the release candidate stage; it still supports Python 3.7.
+SymPy 1.10 was released on 2022-03-06. It still supports Python 3.7, as needed for Sage 9.6.
+
+​https://github.com/sympy/sympy/wiki/release-notes-for-1.10

 Some doctests needs updating.
-
-​https://github.com/sympy/sympy/wiki/release-notes-for-1.10

 From https://github.com/sympy/sympy/runs/5276327276?check_suite_focus=true

@@ -17,7 +17,13 @@
 Got:
     (0, True)
 **********************************************************************
+```
+This appears to be a deliberate change in sympy. We update the doctest.

+
+Some doctests need updating because of changes to expression canonicalization in sympy:
+
+```
 sage -t --random-seed=141799432516026950115879763089480345171 src/sage/manifolds/continuous_map.py
 **********************************************************************
 File "src/sage/manifolds/continuous_map.py", line 1359, in sage.manifolds.continuous_map.ContinuousMap.coord_functions
@@ -28,59 +34,6 @@
      on the Chart (M, (U, V))
 Got:
     Coordinate functions (-U**2/4 + V**2/4, (-U - V)/(U - V), V) on the Chart (M, (U, V))
+```
+We update these doctests. They will fail now with older versions of sympy. However, we do not adjust the version bounds in `build/pkgs/sympy/install-requires.txt`.

-sage -t --random-seed=141799432516026950115879763089480345171 src/sage/manifolds/differentiable/diff_form.py
-**********************************************************************
-File "src/sage/manifolds/differentiable/diff_form.py", line 268, in sage.manifolds.differentiable.diff_form.DiffForm
-Failed example:
-    s.display(eU)
-Expected:
-    a∧b = -x*(2*x*y + 1) dx∧dy
-Got:
-    a∧b = x*(-2*x*y - 1) dx∧dy
-**********************************************************************
-File "src/sage/manifolds/differentiable/diff_form.py", line 277, in sage.manifolds.differentiable.diff_form.DiffForm
-Failed example:
-    s.display(eU)
-Expected:
-    f*a = -y*(x**2 + 2*x*y + y**2) dx + x*(x**2 + 2*x*y + y**2) dy
-Got:
-    f*a = y*(-x**2 - 2*x*y - y**2) dx + x*(x**2 + 2*x*y + y**2) dy
-**********************************************************************
-
-sage -t --random-seed=141799432516026950115879763089480345171 src/sage/manifolds/continuous_map.py
-**********************************************************************
-File "src/sage/manifolds/continuous_map.py", line 1359, in sage.manifolds.continuous_map.ContinuousMap.coord_functions
-Failed example:
-    Phi.coord_functions(c_UV, c_xyz)
-Expected:
-    Coordinate functions (-U**2/4 + V**2/4, -(U + V)/(U - V), V)
-     on the Chart (M, (U, V))
-Got:
-    Coordinate functions (-U**2/4 + V**2/4, (-U - V)/(U - V), V) on the Chart (M, (U, V))
-**********************************************************************
-
-sage -t --random-seed=141799432516026950115879763089480345171 src/sage/symbolic/expression_conversions.py
-**********************************************************************
-File "src/sage/symbolic/expression_conversions.py", line 870, in sage.symbolic.expression_conversions.SympyConverter.derivative
-Failed example:
-    df_sympy == f_sympy.diff(x, 2, y, 1)
-Exception raised:
-    Traceback (most recent call last):
-      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sage/doctest/forker.py", line 694, in _run
-        self.compile_and_execute(example, compiler, test.globs)
-      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
-        exec(compiled, globs)
-      File "<doctest sage.symbolic.expression_conversions.SympyConverter.derivative[5]>", line 1, in <module>
-        df_sympy == f_sympy.diff(x, Integer(2), y, Integer(1))
-      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sympy/core/expr.py", line 3533, in diff
-        return _derivative_dispatch(self, *symbols, **assumptions)
-      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sympy/core/function.py", line 1920, in _derivative_dispatch
-        return Derivative(expr, *variables, **kwargs)
-      File "/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/sympy/core/function.py", line 1345, in __new__
-        raise ValueError(filldedent('''
-    ValueError: 
-    Can't calculate derivative wrt 2.
-**********************************************************************
-```
-
mkoeppe commented 2 years ago

Description changed:

egourgoulhon commented 2 years ago
comment:17

LGTM. Thanks!

egourgoulhon commented 2 years ago

Reviewer: Eric Gourgoulhon

vbraun commented 2 years ago

Changed branch from u/mkoeppe/update_doctests_for_compatibility_with_sympy_1_10 to 6b7c3a2