Opened 4 years ago

Closed 16 months ago

#15046 closed defect (fixed)

Symbolic elliptic integrals

Reported by: eviatarbach Owned by:
Priority: major Milestone: sage-7.3
Component: symbolics Keywords: special, functions
Cc: burcin, paulmasson Merged in:
Authors: Eviatar Bach, Ralf Stephan Reviewers: Karl-Dieter Crisman, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: fc1ac57 (Commits) Commit: fc1ac5749722831ed8c86ac9c12917a5f299e327
Dependencies: Stopgaps:

Description (last modified by rws)

This ticket is for making the elliptic integrals symbolic, including arbitrary-precision numeric evaluation.

Defect because elliptic_e(2.5,2.5).n() throwing an ECL error is fixed with it.

Attachments (1)

trac15046.patch (14.9 KB) - added by eviatarbach 4 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 Changed 4 years ago by eviatarbach

  • Status changed from new to needs_review

Changed 4 years ago by eviatarbach

comment:2 Changed 4 years ago by eviatarbach

  • Status changed from needs_review to needs_work

Need to get full doctest coverage.

comment:3 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:5 Changed 3 years ago by rws

  • Branch set to u/rws/symbolic_elliptic_integrals

comment:6 Changed 3 years ago by rws

  • Authors set to Eviatar Bach
  • Commit set to a922acc1b23180816f5655e77f5030b530370390

Updated to 6.3beta6:

3 items had failures:
   1 of   4 in sage.functions.special.elliptic_j.EllipticEC
   1 of   2 in sage.functions.special.elliptic_j.EllipticEC._derivative_
   1 of   4 in sage.functions.special.elliptic_j.EllipticPi

New commits:

a922acc15046: Making elliptic integrals symbolic

comment:7 Changed 3 years ago by git

  • Commit changed from a922acc1b23180816f5655e77f5030b530370390 to e950c33f11296ba41c8f7cccf41cc92649deb1e8

Branch pushed to git repo; I updated commit sha1. New commits:

67d51c915046: corrections to previous patch to pass doctests
1b3320715046: accept algorithm param for all evalf functions
e950c3315046: add all doctests; fix latex; augment documentation

comment:8 Changed 3 years ago by rws

  • Authors changed from Eviatar Bach to Eviatar Bach, Ralf Stephan
  • Keywords special functions added
  • Status changed from needs_work to needs_review

This is now ready for review.

BTW, is Maxima not able to solve the resp. integrals?

sage: integrate(sqrt(1 - m*sin(x)^2), x, 0, phi)
sage: integrate(sqrt(1 - m*sin(x)^2), x, 0, pi/2)
sage: integrate(1/sqrt(1 - m*sin(x)^2), x, 0, phi)

comment:9 Changed 3 years ago by git

  • Commit changed from e950c33f11296ba41c8f7cccf41cc92649deb1e8 to 5547215bb7849935ff3a356704a584ddda5ccbf5

Branch pushed to git repo; I updated commit sha1. New commits:

5547215Merge branch 'develop' into t/15046/symbolic_elliptic_integrals

comment:10 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 Changed 3 years ago by git

  • Commit changed from 5547215bb7849935ff3a356704a584ddda5ccbf5 to 829da64c2768eb057f9edaa02a2fba80b22564fa

Branch pushed to git repo; I updated commit sha1. New commits:

829da64Merge branch 'develop' into t/15046/symbolic_elliptic_integrals

comment:12 Changed 3 years ago by jdemeyer

Just to let you know: this conflicts with #17130.

comment:13 Changed 3 years ago by git

  • Commit changed from 829da64c2768eb057f9edaa02a2fba80b22564fa to ad02c9ca6c62d728af961435dd7333ef8e0df18f

Branch pushed to git repo; I updated commit sha1. New commits:

d5c9eb7Merge branch 'develop' into t/15046/symbolic_elliptic_integrals
ad02c9c15046: simplifications due to BuiltinFunction improvements

comment:14 Changed 3 years ago by rws

With this MaximaFunction is no longer used. I tend to put it in its own source file in functions. Comments?

comment:15 Changed 3 years ago by kcrisman

With this MaximaFunction? is no longer used. I tend to put it in its own source file in functions.

Do you mean it isn't used at all? Wow! I guess that is a good thing, though.

comment:16 Changed 3 years ago by rws

  • Status changed from needs_review to needs_work
sage -t --long src/sage/functions/special.py  # 2 doctests failed

comment:17 Changed 3 years ago by git

  • Commit changed from ad02c9ca6c62d728af961435dd7333ef8e0df18f to e1e8900ace95033f1718ef2fdb421c1406cabce0

Branch pushed to git repo; I updated commit sha1. New commits:

1b5573bMerge branch 'develop' into t/15046/symbolic_elliptic_integrals
e1e890015046: fix doctests

comment:18 Changed 3 years ago by rws

  • Milestone changed from sage-6.4 to sage-6.6
  • Status changed from needs_work to needs_review

comment:19 Changed 3 years ago by rws

  • Description modified (diff)
  • Type changed from enhancement to defect

comment:20 Changed 3 years ago by git

  • Commit changed from e1e8900ace95033f1718ef2fdb421c1406cabce0 to 79c8fda1cd8a29c7dd435f494be59a5ea6b99c8f

Branch pushed to git repo; I updated commit sha1. New commits:

79c8fda15046: add doctest for Maxima error

comment:21 Changed 3 years ago by git

  • Commit changed from 79c8fda1cd8a29c7dd435f494be59a5ea6b99c8f to b3b2e8cb5d97ec535849747fd207d6ee92524f6f

Branch pushed to git repo; I updated commit sha1. New commits:

f71294aMerge branch 'develop' into t/15046/symbolic_elliptic_integrals
b3b2e8c15046: fix doctest

comment:22 Changed 3 years ago by rws

  • Milestone changed from sage-6.6 to sage-6.7

Passes all patchbot tests.

comment:23 Changed 2 years ago by kcrisman

  • Dependencies #14996 deleted

I'd like to look at this - can you confirm that you are giving positive review to Eviatar's original changes? If so, I think I only need to give this one more good look and try out some more numerical things to confirm it's ready to go.

By the way, I note some inconsistency (probably due to my relative unfamiliarity with elliptic integrals). I see

This returns the value of the "incomplete elliptic integral of the
    second kind"

in two places, EllipticE and EllipticEU. Since the Maxima manual clarifies the relationship, we should too. Indeed, there is a bunch of stuff in here which assumes familiarity with syntax for the Jacobi functions dn or sn or the rest which should be clarified. Maybe a link in internal documentation to that file, or perhaps some Wikipedia refs would also not be amiss? Just throwing out doc ideas, as long as we implement a little more it will be fine.

comment:24 Changed 2 years ago by rws

  • Status changed from needs_review to needs_work

comment:25 Changed 2 years ago by git

  • Commit changed from b3b2e8cb5d97ec535849747fd207d6ee92524f6f to 27278e3a64c6a0bd0ea0c12a8d9b37908bfbd090

Branch pushed to git repo; I updated commit sha1. New commits:

9a62ed2Merge branch 'develop' into t/15046/symbolic_elliptic_integrals
27278e315046: add refs

comment:26 Changed 2 years ago by rws

  • Milestone changed from sage-6.7 to sage-6.8
  • Status changed from needs_work to needs_review

comment:27 Changed 17 months ago by git

  • Commit changed from 27278e3a64c6a0bd0ea0c12a8d9b37908bfbd090 to fa4fd6965d7180f0d5ed129429eb5b540545edbe

Branch pushed to git repo; I updated commit sha1. New commits:

6138b32Merge branch 'develop' into t/15046/symbolic_elliptic_integrals
fa4fd69fix doctest

comment:28 Changed 17 months ago by rws

  • Cc paulmasson added
  • Milestone changed from sage-6.8 to sage-7.3

comment:29 Changed 16 months ago by rws

Sage startup time increased by 0.3%, why?

comment:30 Changed 16 months ago by tscrim

I am pretty sure that is from a bad startup time test batch. Also, could you break the longer lines to ~80 characters?

comment:31 Changed 16 months ago by git

  • Commit changed from fa4fd6965d7180f0d5ed129429eb5b540545edbe to e4b089ad130328607c5ee2bdbc149cee097f1520

Branch pushed to git repo; I updated commit sha1. New commits:

bebff6cMerge branch 'develop' into t/15046/symbolic_elliptic_integrals
e4b089a15046: cosmetics

comment:32 Changed 16 months ago by rws

I didn't break all the lines. If you insist I'll try again. Anything else?

comment:33 Changed 16 months ago by tscrim

  • Reviewers set to Karl-Dieter Crisman, Travis Scrimshaw

I would prefer if the doctest output lines were split, probably 1 term per line should be good (or more if they are shorter). Once you do that, you can set a positive review on my behalf

comment:34 Changed 16 months ago by git

  • Commit changed from e4b089ad130328607c5ee2bdbc149cee097f1520 to fc1ac5749722831ed8c86ac9c12917a5f299e327

Branch pushed to git repo; I updated commit sha1. New commits:

fc1ac5715046: break doctest output lines

comment:35 Changed 16 months ago by rws

  • Status changed from needs_review to positive_review

I couldn't break lines at abc*xyz (bug?) but it worked at abc - xyz and abc + xyz so I succeeded in most of the cases.

comment:36 Changed 16 months ago by tscrim

Thanks. What you did is exactly what I was looking for and plenty good. The reason why abc*xyz cannot split is because there is no whitespace around the operator (and newlines get counted as whitespace, which the doctest framework ignores all after one whitespace char).

Last edited 16 months ago by tscrim (previous) (diff)

comment:37 Changed 16 months ago by vbraun

  • Branch changed from u/rws/symbolic_elliptic_integrals to fc1ac5749722831ed8c86ac9c12917a5f299e327
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.