Opened 2 years ago

Closed 2 years ago

# Allow precision to be passed into the elliptic_j function

Reported by: Owned by: menosgeze minor sage-9.0 elliptic curves elliptic_j, precision, elliptic curves Gerardo E Zelaya Eufemia, Kevin Lui Kevin Lui, Frédéric Chapoton N/A 58ba0e8 58ba0e8ed3f0e007b67b07c3fc9f1c80ca21cffb

### Description

For example, Wikipedia says that the j-invariant 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:

```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.3012822400356887122945119790e-12*I
```

### comment:1 Changed 2 years ago by menosgeze

• Branch set to u/menosgeze/allow_precision_to_be_passed_into_the_elliptic_j_function

### comment:2 Changed 2 years ago by klui

• Commit set to 5bfbc61a629982af447ee8e13b7bbdc6b00afe0a

I see that the PARI function `ellj` also takes in precision. Should we also pass in the precision into that as well?

# Style Suggestions:

• Add extra line before function.
• Delete spaces around equal sign.
• Indentation should be a multiple of 4.

# Doc string suggestions:

• Add prec to the input line of documentation.

### comment:3 Changed 2 years ago by git

• 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 git

• 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 git

• 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 git

• 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 git

• 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 menosgeze

• Authors set to Gerardo E Zelaya Eufemia
• 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 klui

• 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 git

• 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 menosgeze

• Status changed from needs_work to needs_review

### comment:12 Changed 2 years ago by klui

• 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 chapoton

reviewer name is missing

### comment:14 Changed 2 years ago by klui

• Reviewers set to Kevin Lui

Opps. Thanks!

### comment:15 Changed 2 years ago by chapoton

• Milestone changed from sage-8.9 to sage-9.0

moving milestone to 9.0 (after release of 8.9)

### comment:16 Changed 2 years ago by vbraun

• Status changed from positive_review to needs_work

On Ubuntu 10 32-bit:

```**********************************************************************
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.3012822400356887122945119790e-12*I
Got:
2.6253741264076799999999999999e17 - 1.3012822398031636140364186716e-12*I
**********************************************************************
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 klui

• 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 klui

• 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 32-bit machine. I don't have access to a 32-bit 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 32-bit machines`

### comment:19 Changed 2 years ago by chapoton

```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 chapoton

• Status changed from needs_review to needs_work

### comment:21 Changed 2 years ago by klui

• 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 git

• 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 32-bit machines` ​58ba0e8 `Fix doctest location of rel/abs tol flag`

### comment:23 Changed 2 years ago by klui

• Authors changed from Gerardo E Zelaya Eufemia to Gerardo E Zelaya Eufemia, Kevin Lui
• 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 chapoton

• 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 vbraun

• 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
Note: See TracTickets for help on using tickets.