Opened 2 years ago
Closed 2 years ago
#28355 closed enhancement (fixed)
Allow precision to be passed into the elliptic_j function
Reported by:  menosgeze  Owned by:  

Priority:  minor  Milestone:  sage9.0 
Component:  elliptic curves  Keywords:  elliptic_j, precision, elliptic curves 
Cc:  Merged in:  
Authors:  Gerardo E Zelaya Eufemia, Kevin Lui  Reviewers:  Kevin Lui, Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  58ba0e8 (Commits, GitHub, GitLab)  Commit:  58ba0e8ed3f0e007b67b07c3fc9f1c80ca21cffb 
Dependencies:  Stopgaps: 
Description
For example, Wikipedia says that the jinvariant of (1+sqrt(163))/2 is an integer, but if one computes elliptic_j for this symbolic ring element, it returns a complex number with an imaginary part not close to 0. See:
https://en.wikipedia.org/wiki/Jinvariant
sage: b = sqrt(163) sage: b = (1+b)/2 sage: elliptic_j(b) 2.62537412640767e17 + 732.558854258998*I sage: elliptic_j(ComplexField(100(b)) 2.6253741264076799999999999999e17 + 1.3012822400356887122945119790e12*I
Change History (25)
comment:1 Changed 2 years ago by
 Branch set to u/menosgeze/allow_precision_to_be_passed_into_the_elliptic_j_function
comment:2 Changed 2 years ago by
 Commit set to 5bfbc61a629982af447ee8e13b7bbdc6b00afe0a
comment:3 Changed 2 years ago by
 Commit changed from 5bfbc61a629982af447ee8e13b7bbdc6b00afe0a to 30092a2a9daa403c8a593d6c4c061b569d45e771
Branch pushed to git repo; I updated commit sha1. New commits:
30092a2  corrected all klui suggestions except pari input precision

comment:4 Changed 2 years ago by
 Commit changed from 30092a2a9daa403c8a593d6c4c061b569d45e771 to 1f2495fb7f99fcf039df760f0a79ea46b4a63066
Branch pushed to git repo; I updated commit sha1. New commits:
1f2495f  corrected trailing spaces

comment:5 Changed 2 years ago by
 Commit changed from 1f2495fb7f99fcf039df760f0a79ea46b4a63066 to 0076f6baf548cc0560b18c31dd2fb98aaff7ed71
Branch pushed to git repo; I updated commit sha1. New commits:
0076f6b  added example explanation

comment:6 Changed 2 years ago by
 Commit changed from 0076f6baf548cc0560b18c31dd2fb98aaff7ed71 to f4d7ca2bd150ee13179538d59745c2fb80e3182c
Branch pushed to git repo; I updated commit sha1. New commits:
f4d7ca2  corrected explanation of example

comment:7 Changed 2 years ago by
 Commit changed from f4d7ca2bd150ee13179538d59745c2fb80e3182c to bd02eb423fbe35ba58d4f0be7984d0d9b02594ad
Branch pushed to git repo; I updated commit sha1. New commits:
bd02eb4  corrected explanation of example

comment:8 Changed 2 years ago by
 Status changed from new to needs_review
The elliptic_j
function needed an extra precision argument prec
, and I fixed the indentation for the elliptic_j
function.
New commits:
bd02eb4  corrected explanation of example

comment:9 Changed 2 years ago by
 Status changed from needs_review to needs_work
Docstring formatting error in the input section.
Indentation of L309 is off by one.
comment:10 Changed 2 years ago by
 Commit changed from bd02eb423fbe35ba58d4f0be7984d0d9b02594ad to e1751627dd272a3ba54eb549cf5d46d8582d56df
Branch pushed to git repo; I updated commit sha1. New commits:
e175162  corrected line 309 indentation

comment:11 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:12 Changed 2 years ago by
 Status changed from needs_review to positive_review
Looks good to me now. I think this is a useful change.
I think the current patchbot failures aren't relevant here. The tests pass on my machine.
comment:13 Changed 2 years ago by
reviewer name is missing
comment:15 Changed 2 years ago by
 Milestone changed from sage8.9 to sage9.0
moving milestone to 9.0 (after release of 8.9)
comment:16 Changed 2 years ago by
 Status changed from positive_review to needs_work
On Ubuntu 10 32bit:
********************************************************************** File "src/sage/functions/special.py", line 344, in sage.functions.special.elliptic_j Failed example: elliptic_j(tau) Expected: 2.62537412640767e17  732.558854258998*I Got: 2.62537412640767e17  732.558854103088*I ********************************************************************** File "src/sage/functions/special.py", line 346, in sage.functions.special.elliptic_j Failed example: elliptic_j(tau,75) Expected: 2.625374126407680000000e17  0.0001309913593909879441262*I Got: 2.625374126407680000000e17  0.0001309913596969636273570*I ********************************************************************** File "src/sage/functions/special.py", line 348, in sage.functions.special.elliptic_j Failed example: elliptic_j(tau,100) Expected: 2.6253741264076799999999999999e17  1.3012822400356887122945119790e12*I Got: 2.6253741264076799999999999999e17  1.3012822398031636140364186716e12*I ********************************************************************** 1 item had failures: 3 of 12 in sage.functions.special.elliptic_j [125 tests, 3 failures, 3.96 s]  sage t long src/sage/functions/special.py # 3 doctests failed
comment:17 Changed 2 years ago by
 Branch changed from u/menosgeze/allow_precision_to_be_passed_into_the_elliptic_j_function to u/klui/allow_precision_to_be_passed_into_the_elliptic_j_function
comment:18 Changed 2 years ago by
 Commit changed from e1751627dd272a3ba54eb549cf5d46d8582d56df to 843998895cf79a74bd4c087cac221204a8bf7202
 Status changed from needs_work to needs_review
I did a rebase and I added tolerance for the fixing numerical doctests on 32bit machine. I don't have access to a 32bit machine so let's see what the patchbot say.
I'm not too sure about what to do with the ticket fields. Do I remove myself from reviewer and add myself to author? Just leave it as it?
New commits:
0669ee8  added precision argument to elliptic_j

49b80b6  corrected all klui suggestions except pari input precision

5f1348c  corrected trailing spaces

70870f1  added example explanation

d8593a2  corrected explanation of example

9371454  corrected explanation of example

8a813ab  corrected line 309 indentation

8439988  Add a rel tol to pass failing numerical doctests on 32bit machines

comment:19 Changed 2 years ago by
sage t long src/sage/functions/special.py # 3 doctests failed
The tag "rel tol" should be on the command line in the doctest, not on the result line.
You can add yourself as author and stay also reviewer. You need someone else to review your own changes.
comment:20 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:21 Changed 2 years ago by
 Branch changed from u/klui/allow_precision_to_be_passed_into_the_elliptic_j_function to u/klui/allow_precision_to_be_passed_into_the_elliptic_j_function_rebase
 Commit 843998895cf79a74bd4c087cac221204a8bf7202 deleted
comment:22 Changed 2 years ago by
 Commit set to 58ba0e8ed3f0e007b67b07c3fc9f1c80ca21cffb
Branch pushed to git repo; I updated commit sha1. New commits:
e7f6676  added precision argument to elliptic_j

2c20be9  corrected all klui suggestions except pari input precision

da83b87  corrected trailing spaces

fbb4f83  added example explanation

ec1d6f9  corrected explanation of example

6cf6390  corrected explanation of example

7327263  corrected line 309 indentation

5a871a9  Add a rel tol to pass failing numerical doctests on 32bit machines

58ba0e8  Fix doctest location of rel/abs tol flag

comment:23 Changed 2 years ago by
 Status changed from needs_work to needs_review
I did a rebase on develop followed by fixing the location of the tol flags.
comment:24 Changed 2 years ago by
 Reviewers changed from Kevin Lui to Kevin Lui, Frédéric Chapoton
 Status changed from needs_review to positive_review
ok
comment:25 Changed 2 years ago by
 Branch changed from u/klui/allow_precision_to_be_passed_into_the_elliptic_j_function_rebase to 58ba0e8ed3f0e007b67b07c3fc9f1c80ca21cffb
 Resolution set to fixed
 Status changed from positive_review to closed
I see that the PARI function
ellj
also takes in precision. Should we also pass in the precision into that as well?Style Suggestions:
Doc string suggestions: