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

sympy: Problem with integration/differentiation of symbolic functions. #28964

Closed
EmmanuelCharpentier mannequin opened this issue Jan 7, 2020 · 21 comments
Closed

sympy: Problem with integration/differentiation of symbolic functions. #28964

EmmanuelCharpentier mannequin opened this issue Jan 7, 2020 · 21 comments

Comments

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Jan 7, 2020

This ticket fixes several problems related to the conversion of derivatives to and from sympy:

  • the derivative arguments were permuted when converting to sympy (comment:7)
  • the conversion from Sage to sympy did not account for the case of variables with multiple occurences (comment:1)
  • the first argument of the derivative was skipped when converting from sympy (comment:6)
  • sympy returns arguments of type sympy.core.containers.Tuple which was not handled
  • Sage's derivative does not accept tuples, so the arguments must be flattened
  • sympy's integer type is converted to Sage's rational type, but should be converted to Sage's integer type

(Original description below.)


Minimal case of a problem reported on ask.sagemath.org.

sage: reset()
sage: var("x,t")
(x, t)
sage: f=function("f") ## Note : no formal arguments
sage: diff(f(x,t),x)
diff(f(x, t), x)
sage: diff(f(x,t),x).integrate(x)
f(x, t)

As expected. But :

sage: diff(f(x,t),x).integrate(t)
f(x, t)

No, no, no. And no.

Slightly better:

sage: reset()
sage: var("x,t")
(x, t)
sage: f=function("f")(x,t)  ## Note : specified formal arguments.
sage: diff(f(x,t),x)
diff(f(t, x), x)
sage: diff(f(x,t),x).integrate(x)
f(t, x)
sage: diff(f(x,t),x).integrate(t)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/all_cmdline.py in <module>()
----> 1 diff(f(x,t),x).integrate(t)

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.integral (build/cythonized/sage/symbolic/expression.cpp:64575)()
  12389                     R = ring.SR
  12390             return R(integral(f, v, a, b, **kwds))
> 12391         return integral(self, *args, **kwds)
  12392 
  12393     integrate = integral

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/integration/integral.py in integrate(expression, v, a, b, algorithm, hold)
    927         return integrator(expression, v, a, b)
    928     if a is None:
--> 929         return indefinite_integral(expression, v, hold=hold)
    930     else:
    931         return definite_integral(expression, v, a, b, hold=hold)

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/function.pyx in sage.symbolic.function.BuiltinFunction.__call__ (build/cythonized/sage/symbolic/function.cpp:12262)()
   1136             res = self._evalf_try_(*args)
   1137             if res is None:
-> 1138                 res = super(BuiltinFunction, self).__call__(
   1139                         *args, coerce=coerce, hold=hold)
   1140 

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/function.pyx in sage.symbolic.function.Function.__call__ (build/cythonized/sage/symbolic/function.cpp:7039)()
    600                     (<Expression>args[0])._gobj, hold)
    601         elif self._nargs == 2:
--> 602             res = g_function_eval2(self._serial, (<Expression>args[0])._gobj,
    603                     (<Expression>args[1])._gobj, hold)
    604         elif self._nargs == 3:

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/integration/integral.py in _eval_(self, f, x)
    100         for integrator in self.integrators:
    101             try:
--> 102                 A = integrator(f, x)
    103             except (NotImplementedError, TypeError, AttributeError):
    104                 pass

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/integration/external.py in sympy_integrator(expression, v, a, b)
     66     else:
     67         result = sympy.integrate(ex, (v, a._sympy_(), b._sympy_()))
---> 68     return result._sage_()
     69 
     70 def mma_free_integrator(expression, v, a=None, b=None):

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/interfaces/sympy.py in _sympysage_integral(self)
    307     """
    308     from sage.misc.functional import integral
--> 309     f, limits = self.function._sage_(), list(self.limits)
    310     for limit in limits:
    311         if len(limit) == 1:

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/interfaces/sympy.py in _sympysage_derivative(self)
    333     f = self.args[0]._sage_()
    334     args = [[a._sage_() for a in arg] if isinstance(arg,tuple) else arg._sage_() for arg in self.args[2:]]
--> 335     return derivative(f, *args)
    336 
    337 def _sympysage_order(self):

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/calculus/functional.py in derivative(f, *args, **kwds)
    129     """
    130     try:
--> 131         return f.derivative(*args, **kwds)
    132     except AttributeError:
    133         pass

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.derivative (build/cythonized/sage/symbolic/expression.cpp:25806)()
   4186             ValueError: No differentiation variable specified.
   4187         """
-> 4188         return multi_derivative(self, args)
   4189 
   4190     diff = differentiate = derivative

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/misc/derivative.pyx in sage.misc.derivative.multi_derivative (build/cythonized/sage/misc/derivative.c:3014)()
    217     if not args:
    218         # fast version where no arguments supplied
--> 219         return F._derivative()
    220 
    221     for arg in derivative_parse(args):

/usr/local/sage-P3-2/local/lib/python3.7/site-packages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression._derivative (build/cythonized/sage/symbolic/expression.cpp:26148)()
   4248                 return self.gradient()
   4249             else:
-> 4250                 raise ValueError("No differentiation variable specified.")
   4251         if not isinstance(deg, (int, long, sage.rings.integer.Integer)) \
   4252                 or deg < 1:

ValueError: No differentiation variable specified.

No again. But at least, this is an explicit error, not a silently accepted wrong result.

The problem seems to point to handling formal arguments of undefined functions. I do not (yet) understand this code...

IMHO, at least the first case is critical (possibly blocker), because it silently returns a wrong result.

Note that the handling of defined functions seems to be correct. For example:

sage: sin(t*x).diff(x)
t*cos(t*x)
sage: sin(t*x).diff(x).integrate(x)
sin(t*x)
sage: sin(t*x).diff(x).integrate(t)
(t*x*sin(t*x) + cos(t*x))/x^2

Component: symbolics

Keywords: integrate, integral, sympy

Author: Markus Wageringel

Branch/Commit: 57a8e83

Reviewer: Vincent Delecroix

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

@EmmanuelCharpentier EmmanuelCharpentier mannequin added this to the sage-9.1 milestone Jan 7, 2020
@nbruin
Copy link
Contributor

nbruin commented Jan 7, 2020

comment:1

Weird. Round-tripping to maxima seems to do the right thing, so that's not where the problem seems to lie:

sage: diff(f(x,t),x)._maxima_().integrate(t)._sage_()
integrate(diff(f(x, t), x), t)

If you make the variable combinations a little more complicated:

sage: diff(f(x,x,t),x).integrate(t)
ValueError: No differentiation variable specified.
sage: diff(f(x,x,t),t).integrate(x)
ValueError: 
Since there is more than one variable in the expression, the
variable(s) of differentiation must be supplied to differentiate f(x,
x, t)

These are Pynac errors. So I suspect that there is a differentiation that doesn't have its variable specified, and that with only two variables around (one for the integration!) it ends up choosing that variable. So my guess is either Pynac itself or the way we interface with it.

The same problem seems to arise the other way around as well:

sage: integrate(f(x,t),x).diff(t)
f(x, t)

we can make that cause an error by fiddling with some more variables and then we see that the error traces back in something being called from _sympysage_integral(), so I think this error was introduced relatively recently, when sympy was added as an integrator by default.

@EmmanuelCharpentier
Copy link
Mannequin Author

EmmanuelCharpentier mannequin commented Jan 7, 2020

comment:2

Replying to @nbruin:

[ Snip... ]

we can make that cause an error by fiddling with some more variables and then we see that the error traces back in something being called from _sympysage_integral(), so I think this error was introduced relatively recently, when sympy was added as an integrator by default.

According to a comment posted around Jan 7, 2020 07:00 by dsejas in the original ask.sagemath question, the problem might have been introduced between 8.8 and 8.9.

It should be possible to git bisect this range to pinpoint the commit. I don't have the necessary resources on hand...

@egourgoulhon
Copy link
Member

comment:3

Replying to @EmmanuelCharpentier:

we can make that cause an error by fiddling with some more variables and then we see that the error traces back in something being called from _sympysage_integral(), so I think this error was introduced relatively recently, when sympy was added as an integrator by default.

According to a comment posted around Jan 7, 2020 07:00 by dsejas in the original ask.sagemath question, the problem might have been introduced between 8.8 and 8.9.

The culprit might be #27958.

@egourgoulhon
Copy link
Member

Changed keywords from none to integrate, integral

@bryanso
Copy link
Mannequin

bryanso mannequin commented Jan 7, 2020

comment:5

I am the original poster in ask.sagemath.org for this issue. Hope I am not out of line to add a test case because I hope the eventual fix will solve my original problem:

var('x t')
f = function('f')(x, t)
g = function('g')(x, t)
integrate(g * diff(f, x), x)
...
ValueError: No differentiation variable specified.

Used to work in Sage 8.8. Not working in 8.9 or 9.0.

Thanks a lot.

@nbruin
Copy link
Contributor

nbruin commented Jan 7, 2020

comment:6

Translation of derivatives to sympy is definitely wonky:

sage: f=function('f')
sage: F=f(x,y,t).diff(t)
sage: F
diff(f(x, y, t), t)
sage: F._sympy_()
Derivative(f(x, y, t), y)
sage: F=f(x,x,t).diff(t)
sage: F._sympy_()
ValueError: 
Since there is more than one variable in the expression, the
variable(s) of differentiation must be supplied to differentiate f(x,
x, t)

There are typos in sage/interfaces/sympy.py:

def _sympysage_derivative(self):
    """
    EXAMPLES::

        sage: from sympy import Derivative
        sage: f = function('f')
        sage: sympy_diff = Derivative(f(x)._sympy_(), x._sympy_())
        sage: assert diff(f(x),x)._sympy_() == sympy_diff
        sage: assert diff(f(x),x) == sympy_diff._sage_()
    """
    from sage.calculus.functional import derivative
    f = self.args[0]._sage_()
    args = [[a._sage_() for a in arg] if isinstance(arg,tuple) else arg._sage_() for arg in self.args[2:]]
    return derivative(f, *args)

I'm pretty sure the second-last line should be iterating over self.args[1:], but changing that doesn't actually seem to fix the problem.

@mwageringel
Copy link

comment:7

Replying to @nbruin:

Translation of derivatives to sympy is definitely wonky:

sage: f=function('f')
sage: F=f(x,y,t).diff(t)
sage: F
diff(f(x, y, t), t)
sage: F._sympy_()
Derivative(f(x, y, t), y)
sage: F=f(x,x,t).diff(t)
sage: F._sympy_()
ValueError: 
Since there is more than one variable in the expression, the
variable(s) of differentiation must be supplied to differentiate f(x,
x, t)

The following fixes this particular problem:

--- a/src/sage/symbolic/expression_conversions.py
+++ b/src/sage/symbolic/expression_conversions.py
@@ -860,7 +860,7 @@ class SympyConverter(Converter):
         # retrieve order
         order = operator._parameter_set
         # arguments
-        _args = ex.arguments()
+        _args = ex.operands()

         sympy_arg = []
         for i, a in enumerate(_args):

arguments contains the operands, but in a different order – alphabetically I assume.

@mwageringel

This comment has been minimized.

@mwageringel mwageringel changed the title Problem with integration/differentiation of symbolic functions. sympy: Problem with integration/differentiation of symbolic functions. Feb 17, 2020
@mwageringel
Copy link

comment:9

The patches in comment:6 and comment:7 almost fix the problem, but there were a few more issues in the conversion of derivatives between sympy and Sage. These are now fixed by this branch.

Most notably, sympy's arguments are encoded as sympy.core.containers.Tuple which was missed in a typecheck isinstance(arg, tuple), and Sage's derivative does not accept tuples as arguments, so they need to be flattened.

Moreover, the conversion from Sage to sympy was very broken and has been rewritten completely, especially to account for the case of variables with multiple occurences as in comment:1.

As far as I can see, all the test cases mentioned on this ticket work correctly now.


New commits:

42d400b28964: fix sympy conversions of derivative
0bfd84028964: sympy: derivatives by variables with multiple occurences

@mwageringel
Copy link

Commit: 0bfd840

@mwageringel
Copy link

Author: Markus Wageringel

@mwageringel
Copy link

Branch: u/gh-mwageringel/28964

@mwageringel
Copy link

Changed keywords from integrate, integral to integrate, integral, sympy

@videlec
Copy link
Contributor

videlec commented Feb 17, 2020

comment:10

Handling the Rational this way leads to problem

sage: cos(x).diff(x, 3/2)
-sin(x)

If this is really needed (I don't quite understand why yet), then you should check that there is no denominator.

@videlec
Copy link
Contributor

videlec commented Feb 17, 2020

Reviewer: Vincent Delecroix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 17, 2020

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

57a8e8328964: fix conversion of integers from sympy to sage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 17, 2020

Changed commit from 0bfd840 to 57a8e83

@mwageringel

This comment has been minimized.

@mwageringel
Copy link

comment:13

You are right. That was not a good solution. It turns out the problem is that elements of type sympy.core.numbers.Integer (a subtype of sympy.core.numbers.Rational) are converted to Sage's Rational. For example, this causes problems here:

sage: import sympy
sage: sympy.sympify('Derivative(f(x, t), x, x)')
Derivative(f(x, t), (x, 2))
sage: _._sage_()            # the 2 is converted to Rational here
...

I have just fixed this by implementing the missing conversion routine for integers. Thanks for pointing me in the right direction. I will squash the commits if you prefer.

@videlec
Copy link
Contributor

videlec commented Feb 18, 2020

comment:14

Good enough as it is.

@vbraun
Copy link
Member

vbraun commented Feb 21, 2020

Changed branch from u/gh-mwageringel/28964 to 57a8e83

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants