Opened 3 years ago

Closed 4 months ago

Last modified 4 months ago

#25119 closed defect (fixed)

Fail to integrate sqrt(x^2)/x

Reported by: rws Owned by:
Priority: major Milestone: sage-9.2
Component: calculus Keywords: integral
Cc: slelievre, kcrisman Merged in:
Authors: Frédéric Chapoton Reviewers: Karl-Dieter Crisman
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: 744d626 (Commits) Commit: 744d62637e46805dff3723cebb5b4d30017f370d
Dependencies: Stopgaps:

Description (last modified by kcrisman)

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.

Change History (12)

comment:1 Changed 4 months ago by chapoton

  • Authors set to Frédéric Chapoton
  • Branch set to u/chapoton/25119
  • Commit set to 744d62637e46805dff3723cebb5b4d30017f370d
  • Keywords integral added
  • Milestone changed from sage-8.2 to sage-9.3
  • Status changed from new to needs_review

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


New commits:

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

comment:2 Changed 4 months ago by chapoton

  • Cc slelievre kcrisman added

bots are morally green, please review

comment:3 Changed 4 months ago by kcrisman

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.

comment:4 Changed 4 months ago by kcrisman

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

comment:5 Changed 4 months ago by kcrisman

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'?

comment:6 Changed 4 months ago by kcrisman

  • Description modified (diff)
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:7 Changed 4 months ago by chapoton

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.

comment:8 Changed 4 months ago by kcrisman

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.

comment:9 Changed 4 months ago by chapoton

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

comment:10 Changed 4 months ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to positive_review

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.

comment:11 Changed 4 months ago by vbraun

  • Branch changed from u/chapoton/25119 to 744d62637e46805dff3723cebb5b4d30017f370d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:12 Changed 4 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.2
Note: See TracTickets for help on using tickets.