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

Fail to integrate sqrt(x^2)/x #25119

Closed
rwst opened this issue Apr 8, 2018 · 17 comments
Closed

Fail to integrate sqrt(x^2)/x #25119

rwst opened this issue Apr 8, 2018 · 17 comments

Comments

@rwst
Copy link

rwst commented Apr 8, 2018

sage: integrate(sqrt(x^2)/x,x)
...
RuntimeError: ECL says: Error executing code in Maxima: expt: undefined: 0 to a negative exponent.

sage: integrate(sqrt(x^2)/x,x,algorithm='fricas')
sqrt(x^2)
sage: integrate(sqrt(x^2)/x,x,algorithm='giac')
x*sign(x)
sage: integrate(sqrt(x^2)/x,x,algorithm='sympy')
sqrt(x^2)

See Maxima bug 3657.

Upstream: Reported upstream. No feedback yet.

CC: @slel @kcrisman

Component: calculus

Keywords: integral

Author: Frédéric Chapoton

Branch/Commit: 744d626

Reviewer: Karl-Dieter Crisman

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

@rwst rwst added this to the sage-8.2 milestone Apr 8, 2018
@fchapoton
Copy link
Contributor

Author: Frédéric Chapoton

@fchapoton
Copy link
Contributor

Branch: u/chapoton/25119

@fchapoton
Copy link
Contributor

Changed keywords from none to integral

@fchapoton
Copy link
Contributor

comment:1

Here is a fix (bandaid). One should report upstream to maxima.


New commits:

744d626fix some details in integration, make one more integral work

@fchapoton
Copy link
Contributor

Commit: 744d626

@fchapoton
Copy link
Contributor

comment:2

bots are morally green, please review

@kcrisman
Copy link
Member

comment:3

Thanks, Frédéric. Can I ask whether the changes other than the new doctest and the addition of RunTimeError are nontrivial? I don't think so, but there were a lot of prettification changes.

@kcrisman
Copy link
Member

comment:4

In vanilla Maxima:

(%i4) domain:complex;
(%o4)                               complex
(%i5) integrate(sqrt(x^2)/x,x);

expt: undefined: 0 to a negative exponent.
 -- an error. To debug this try: debugmode(true);

@kcrisman
Copy link
Member

comment:5

However, before giving positive review, I'd suppose we'd want a way to check that this one was fixed - maybe # known bug below it where we require algorithm='maxima'?

@kcrisman

This comment has been minimized.

@kcrisman
Copy link
Member

Upstream: Reported upstream. No feedback yet.

@fchapoton
Copy link
Contributor

comment:7

All the other changes are purely white space removal or addition, for the sake of flake8 conmpliance.

I guess one could a "known bug" doctest, yes.

@kcrisman
Copy link
Member

comment:8

Ah yes. I haven't used that, personally, but I'm sure it complains a lot. Unfortunately, it just makes tickets harder to review at times. I won't raise this on -devel because I know how annoyingly much extra work it would be, but having two different commits for that sort of thing is helpful to reviewers.

@fchapoton
Copy link
Contributor

comment:9

Do you want the "known bug" doctest ? This does not seem to be really necessary to me. We are not responsible for maxima bugs.

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member

comment:10

It would be nice, because we typically do this in other cases. But I guess since in this case it is an actual exception raised, as opposed to a wrong result we had to work around, it is not necessary.

But if Volker complains about failing doctests, I am trusting your morally green patchbot :-) Just upgraded OS (still several versions behind) and so won't be building new Sage for a little while until I have time to check that command line tools are working properly.

@vbraun
Copy link
Member

vbraun commented Sep 27, 2020

Changed branch from u/chapoton/25119 to 744d626

@vbraun vbraun closed this as completed in 85906ba Sep 27, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.2 Sep 27, 2020
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