sagemath / sage

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

Definite integral should not depend on the dummy variable #12438

Closed novoselt closed 12 years ago

novoselt commented 12 years ago

Consider the following:

sage: f(x) = x; f
x |--> x
sage: integral(f, x)
x |--> 1/2*x^2
sage: integral(f, x, 0, 1)
x |--> 1/2

The last integral has not "happened" to be constant - it does not depend on x mathematically, so it should not depend on x in Sage.

Multivariate case:

sage: f(x, y) = x + y
sage: f
(x, y) |--> x + y
sage: integral(f, x, 0, 1)
(x, y) |--> y + 1/2
sage: _(3)
y + 1/2 

integral(...) here should return a symbolic function that depends on y only, so that the last evaluation gives 7/2 (and there are no warning about unnamed evaluation - the order of variables is the same as for the original function with one variable dropped).

See discussion here: http://groups.google.com/group/sage-devel/browse_thread/thread/1309eeae0714be79

CC: @kcrisman @jasongrout

Component: symbolics

Keywords: sd40.5

Author: Andrey Novoseltsev

Reviewer: Karl-Dieter Crisman, Benjamin Jones, Douglas McNeil

Merged: sage-5.1.beta3

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

novoselt commented 12 years ago
comment:2

Working on it.

novoselt commented 12 years ago

Changed keywords from none to sd40.5

kcrisman commented 12 years ago
comment:3

After rereading the original thread, as discussed in person, I think that extracting all the (callable) variables from the original expression and then returning a callable function with all but one of them in the same order (is this easy to do?) should be doable and ok.

novoselt commented 12 years ago
comment:4

The offending lines are

    if is_CallableSymbolicExpressionRing(self._parent):
        return self._parent(integral(ring.SR(self), *args, **kwds))

So if we start with at function depending on x and y, the result will also be a function depending on x and y.

The question is now in distinguishing definite and indefinite integrals, as they have to be treated differently.

novoselt commented 12 years ago

Author: Andrey Novoseltsev

novoselt commented 12 years ago
comment:5

Done!

novoselt commented 12 years ago
comment:6

First three hunks replace TABs with spaces - I thought they were gone from the library?..

kcrisman commented 12 years ago
comment:7

This looks pretty good. Running tests.

kcrisman commented 12 years ago

Reviewer: Karl-Dieter Crisman

56048686-9665-4c5e-973e-6c3add3aa805 commented 12 years ago
comment:8

Just so it's recorded:

sage: f(x) = x
sage: var("y")
y
sage: integral(f, y, 0, 3)
ERROR: An unexpected error occurred while tokenizing input
The following traceback may be corrupted or invalid
The error message is: ('EOF in multi-line statement', (580, 0))

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)

/home/mcneil/sagedev/sage-5.1.beta0/devel/sage-hack/sage/<ipython console> in <module>()

/home/mcneil/sagedev/sage-5.1.beta0/local/lib/python2.7/site-packages/sage/misc/functional.pyc in integral(x, *args, **kwds)
    726     """
    727     if hasattr(x, 'integral'):
--> 728         return x.integral(*args, **kwds)
    729     else:
    730         from sage.symbolic.ring import SR

/home/mcneil/sagedev/sage-5.1.beta0/local/lib/python2.7/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.integral (sage/symbolic/expression.cpp:33895)()

ValueError: list.remove(x): x not in list
kcrisman commented 12 years ago
comment:9

Reviewer patch fixes one doctest - should be able to apply even after fixing this.

kcrisman commented 12 years ago
comment:10

Replying to @sagetrac-dsm:

Just so it's recorded:

> sage: f(x) = x
> sage: var("y")

But

sage: var('y')   
y
sage: integral(x,y,0,3)
3*x

so it's only that branch which causes problems.

novoselt commented 12 years ago
comment:11

No more problems and extra doctests are added!

Apply only trac_12438_drop_variable_of_definite_integration.patch

novoselt commented 12 years ago
comment:12

Fixed "OUPUT" typo.

benjaminfjones commented 12 years ago
comment:13

The most recent patch applies with fuzz to sage-5.0:

Hunk #3 succeeded at 653 with fuzz 1 (offset -11 lines).

but maybe it applies cleanly to 5.1.beta0. Relevant tests pass in sage/calculus, sage/symbolic, and sage/functions. I'm running a patchbot instance on it for the complete test suite.

benjaminfjones commented 12 years ago

Changed reviewer from Karl-Dieter Crisman to Karl-Dieter Crisman, Benjamin Jones

novoselt commented 12 years ago
comment:15

Bloody patchbot, I repeat:

Apply only trac_12438_drop_variable_of_definite_integration.patch

(And I am working on 5.1.beta0, so hopefully there will be no fuzz during merging.)

kcrisman commented 12 years ago

Changed reviewer from Karl-Dieter Crisman, Benjamin Jones to Karl-Dieter Crisman, Benjamin Jones, Douglas McNeil

kcrisman commented 12 years ago
comment:16

Replying to @benjaminfjones:

The most recent patch applies with fuzz to sage-5.0:

Hunk #3 succeeded at 653 with fuzz 1 (offset -11 lines).

but maybe it applies cleanly to 5.1.beta0.

Correct, it does.

So we're set to go?

benjaminfjones commented 12 years ago
comment:17

The 5.1.beta0 patchbot says tests pass, so I think we're good here. Positive review.

novoselt commented 12 years ago

Attachment: trac_12438_drop_variable_of_definite_integration.patch.gz

Apply this patch only

novoselt commented 12 years ago
comment:18

2607 was fixing the same TABs as this ticket, I've rebased this one on top of that one. No code was changed, so I'll leave it at positive review.

novoselt commented 12 years ago

Dependencies: #2607

novoselt commented 12 years ago

Changed dependencies from #2607 to none

novoselt commented 12 years ago
comment:19

I've realized that since I've just removed conflicting hunks this patch has become independent of #2607.

jdemeyer commented 12 years ago

Merged: sage-5.1.beta3

kcrisman commented 12 years ago
comment:21

On a related note, Sage has gone the other way on limits.

$ Downloads/sage-4.8/sage
----------------------------------------------------------------------
| Sage Version 4.8, Release Date: 2012-01-20                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
sage: f(x) = x^2
sage: limit(f,x=1)
x |--> 1
sage: limit(f(x),x=1)
1
sage: 
Exiting Sage (CPU time 0m1.15s, Wall time 0m10.57s).
$ Desktop/sage-4.4.4-mcbc/sage
----------------------------------------------------------------------
| Sage Version 4.4.4, Release Date: 2010-06-23                       |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
Loading Sage library. Current Mercurial branch is: hackbranch
sage: f(x) = x^2
sage: limit(f,x=1)
1
sage: limit(f(x),x=1)
1

This seems bad, so I've opened #13221 for that.