-
-
Notifications
You must be signed in to change notification settings - Fork 580
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
integral of abs(cos(x))*sin(x) gives false results #8624
Comments
comment:1
This might be related to this bug, which should be somewhere on trac:
|
comment:2
#8729 may point to a solution |
comment:4
This patch fixes the problem, but introduces some other doctest errors that should be fixed. |
Author: Jason Grout |
comment:5
Make sure it doesn't introduce any errors - sometimes loading extra Maxima packages causes trouble. Also note that elsewhere there are complaints about Maxima start time, which this would contribute to. |
comment:6
Replying to @kcrisman:
Sure, but it was wrong before, and correctness trumps maxima startup time. Unless we can detect what kind of integrals need this package loaded and load it dynamically, I think the best thing is to load it up front. |
Attachment: trac-8624-abs-integration.patch.gz |
comment:7
I think I caught the failing doctests... |
comment:8
I don't like replacing the z85 etc. with z... stuff. The output is not random, it just changes whenever we change those doctests. Until we find a way to have Sage parse that and replace it with our own variables (if we even want to do that, which I'm not convinced of), it seems reasonable to just change them. |
Reviewer: Burcin Erocal |
comment:9
It's exciting to see that we can handle one more of the Wester tests. Thanks for the patch Jason. I get the following errors after applying attachment: trac-8624-abs-integration.patch to 4.4.2:
Maple simply gives 2 for this one:
Here's the Maple output:
It would be interesting to see how this is transformed to the expression with the incomplete gamma function above.
This just distributes the integral to the polynomial in the numerator. It's interesting to see that maxima cannot handle results with algebraic numbers.
Here is the result from Maple:
This looks OK to me.
We saw this in |
comment:10
Hmm, did you have the new Maxima package at #8731 already installed? This is dealt with there.
Apparently via Gamma(-1,x)=Ei(-x)+e^(-x)/x+1/2 (log(-1/x)-log(-x))+log(x) and the connection between Ei and Ci. But it does check out! |
comment:11
This is actually ok, because it is supposed to return an antiderivative, not a definite integral. It is fantastically more complicated than it has to be, but it would simplify to
which is indeed the correct antiderivative.
Which is clearly correct, and indeed given by the previous line in the file:
So if this really is all the errors (I will check this with the new Maxima package momentarily), then I would say positive review once the z... are reverted to actual numbers. I thought of another reason for this - the user reading documentation might be confused about that if they didn't see the actual output. |
comment:12
One more thing; this patch will have a failure in doctest due to the semicolon in line 334 of calculus/calculus.py, which suppresses the output in Sage, of course. Otherwise I think that (with #8731) this will be a good improvement. |
comment:13
This is fixed at ticket #10187 by upgrading Maxima to version 5.22.1:
Mathematica 6.0 also agrees:
So I'm closing this ticket as fixed. |
comment:14
Is that doctested in the patches for #10187? |
comment:15
What about the other integrals that the patch adds to the doctests? Do those integrals work now too? If not, we should reopen this ticket or make a new one for those integrals. |
comment:16
Replying to @jasongrout:
No. But it shouldn't be difficult to write a documentation patch with doctests in the current ticket. The Sage 4.6.1 release cycle is now in feature freeze, but I think documentation patches are OK for merging in the upcoming release candidates. See #10434 for a follow-up documentation ticket. |
comment:17
So... how's 'bout reopenin' this ticket with a changed title as suggested at #10434? I don't want to get in trouble with someone, but I might do it myself... |
comment:18
See #11483, since reopening is now not allowed for non-release managers :) |
The integral of abs(cos(x))*sin(x) returns the result as if abs() is ignored:
while
CC: @kcrisman
Component: calculus
Author: Jason Grout
Reviewer: Burcin Erocal
Issue created by migration from https://trac.sagemath.org/ticket/8624
The text was updated successfully, but these errors were encountered: