Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#12438 closed defect (fixed)

Definite integral should not depend on the dummy variable

Reported by: novoselt Owned by: burcin
Priority: major Milestone: sage-5.1
Component: symbolics Keywords: sd40.5
Cc: kcrisman, jason Merged in: sage-5.1.beta3
Authors: Andrey Novoseltsev Reviewers: Karl-Dieter Crisman, Benjamin Jones, Douglas McNeil
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

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

Attachments (1)

trac_12438_drop_variable_of_definite_integration.patch (6.8 KB) - added by novoselt 7 years ago.
Apply this patch only

Download all attachments as: .zip

Change History (22)

comment:1 Changed 7 years ago by kcrisman

  • Cc kcrisman jason added

comment:2 Changed 7 years ago by novoselt

  • Keywords sd40.5 added

Working on it.

comment:3 Changed 7 years ago by kcrisman

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.

comment:4 Changed 7 years ago by novoselt

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.

comment:5 Changed 7 years ago by novoselt

  • Authors set to Andrey Novoseltsev
  • Status changed from new to needs_review

Done!

comment:6 Changed 7 years ago by novoselt

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

comment:7 Changed 7 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman

This looks pretty good. Running tests.

comment:8 follow-up: Changed 7 years ago by dsm

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

comment:9 Changed 7 years ago by kcrisman

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

comment:10 in reply to: ↑ 8 Changed 7 years ago by kcrisman

Replying to 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.

comment:11 Changed 7 years ago by novoselt

No more problems and extra doctests are added!

Apply only trac_12438_drop_variable_of_definite_integration.patch

comment:12 Changed 7 years ago by novoselt

Fixed "OUPUT" typo.

comment:13 follow-up: Changed 7 years ago by 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. Relevant tests pass in sage/calculus, sage/symbolic, and sage/functions. I'm running a patchbot instance on it for the complete test suite.

comment:14 Changed 7 years ago by benjaminfjones

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Benjamin Jones

comment:15 Changed 7 years ago by novoselt

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.)

comment:16 in reply to: ↑ 13 Changed 7 years ago by kcrisman

  • Reviewers changed from Karl-Dieter Crisman, Benjamin Jones to Karl-Dieter Crisman, Benjamin Jones, Douglas McNeil

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?

comment:17 Changed 7 years ago by benjaminfjones

  • Status changed from needs_review to positive_review

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

Changed 7 years ago by novoselt

Apply this patch only

comment:18 Changed 7 years ago by novoselt

  • Dependencies set to #2607

#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.

comment:19 Changed 7 years ago by novoselt

  • Dependencies #2607 deleted

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

comment:20 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.1.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:21 Changed 7 years ago by kcrisman

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.

Note: See TracTickets for help on using tickets.