Ticket #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 | Work issues: | |
| Report Upstream: | N/A | Reviewers: | Karl-Dieter Crisman, Benjamin Jones, Douglas McNeil |
| Authors: | Andrey Novoseltsev | Merged in: | sage-5.1.beta3 |
| 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
Change History
comment:3 Changed 12 months 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 12 months 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 12 months ago by novoselt
- Status changed from new to needs_review
- Authors set to Andrey Novoseltsev
Done!
comment:6 Changed 12 months ago by novoselt
First three hunks replace TABs with spaces - I thought they were gone from the library?..
comment:7 Changed 12 months ago by kcrisman
- Reviewers set to Karl-Dieter Crisman
This looks pretty good. Running tests.
comment:8 follow-up: ↓ 10 Changed 12 months 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 12 months ago by kcrisman
Reviewer patch fixes one doctest - should be able to apply even after fixing this.
comment:10 in reply to: ↑ 8 Changed 12 months 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 12 months ago by novoselt
No more problems and extra doctests are added!
Apply only trac_12438_drop_variable_of_definite_integration.patch
comment:12 Changed 12 months ago by novoselt
Fixed "OUPUT" typo.
comment:13 follow-up: ↓ 16 Changed 12 months 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 12 months ago by benjaminfjones
- Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Benjamin Jones
comment:15 Changed 12 months 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 12 months 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 12 months 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 12 months ago by novoselt
-
attachment
trac_12438_drop_variable_of_definite_integration.patch
added
Apply this patch only
comment:18 Changed 12 months 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 12 months 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 12 months ago by jdemeyer
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-5.1.beta3
comment:21 Changed 11 months 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.
