Opened 6 years ago
Closed 6 years ago
#21703 closed enhancement (fixed)
Interface PARI precision in bits
Reported by:  Jeroen Demeyer  Owned by:  

Priority:  major  Milestone:  sage7.5 
Component:  interfaces  Keywords:  
Cc:  Luca De Feo  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Luca De Feo 
Report Upstream:  N/A  Work issues:  
Branch:  a7099ae (Commits, GitHub, GitLab)  Commit:  a7099ae741973f0db3a767c8af1f57989ddacf15 
Dependencies:  Stopgaps: 
Description (last modified by )
PARI now stores its precision internally in bits (as opposed to decimal). This ticket adds an interface for the PARI precision in bits. It also removes the PariInstance._real_precision
attribute and rewrites the documentation about PARI precision.
Change History (31)
comment:1 Changed 6 years ago by
Description:  modified (diff) 

comment:2 Changed 6 years ago by
Branch:  → u/jdemeyer/ticket/21703 

comment:3 Changed 6 years ago by
Authors:  → Jeroen Demeyer 

Commit:  → 313b785064eb21e16d55855c4c2216fa26ca8df0 
Status:  new → needs_review 
comment:4 Changed 6 years ago by
Very interesting, I learned a lot on real precision in PARI.
I'm good with it, but wouldn't the "guide" be best put in __init__
, so that it would be read by anyone who types pari?
?
comment:6 followup: 7 Changed 6 years ago by
set_real_precision()
returns the old precision. set_real_precision_bits()
doesn't return anything. Couldn't we be more consistent here?
comment:7 followup: 8 Changed 6 years ago by
Replying to defeo:
set_real_precision()
returns the old precision.set_real_precision_bits()
doesn't return anything. Couldn't we be more consistent here?
I decided to not return the old value in set_real_precision_bits()
because the PARI call does not return it either. If users really want the old value, they can still call get_real_precision_bits()
.
I did not change set_real_precision()
because we cannot do that without a deprecation.
comment:8 followup: 9 Changed 6 years ago by
Replying to jdemeyer:
Replying to defeo:
set_real_precision()
returns the old precision.set_real_precision_bits()
doesn't return anything. Couldn't we be more consistent here?I decided to not return the old value in
set_real_precision_bits()
because the PARI call does not return it either. If users really want the old value, they can still callget_real_precision_bits()
.I did not change
set_real_precision()
because we cannot do that without a deprecation.
That was my guess. Let's put the deprecation, then?
comment:9 Changed 6 years ago by
Replying to defeo:
That was my guess. Let's put the deprecation, then?
Personally, I wouldn't bother. I don't see the harm in keeping both functions (with a slightly different interface).
comment:10 followup: 11 Changed 6 years ago by
Sure, that's no big deal.
What about the docstrings?
comment:11 Changed 6 years ago by
Replying to defeo:
What about the docstrings?
I have to try to see what works. Ideally, I want the documentation easily findable both from the command line as well as when browsing the reference manual.
comment:12 Changed 6 years ago by
Commit:  313b785064eb21e16d55855c4c2216fa26ca8df0 → b70ec86f4f5aed9152123bb79fc5981e743cd19c 

comment:13 Changed 6 years ago by
Reviewers:  → Luca De Feo 

Status:  needs_review → positive_review 
Ok, these fixes to the docs work *a minima*. If needed, we'll have time to rethink the organization of the docs when we externalize.
comment:14 Changed 6 years ago by
Weird, the startup modules plugin is failing. Presumably not related to this?
comment:16 Changed 6 years ago by
Status:  positive_review → needs_work 

On 32 bit
4014sage t long src/sage/libs/pari/pari_instance.pyx 4015********************************************************************** 4016File "src/sage/libs/pari/pari_instance.pyx", line 185, in sage.libs.pari.pari_instance 4017Failed example: 4018 pari(1).sin(precision=150).sage() 4019Expected: 4020 0.841470984807896506652502321630298999622563060798371065673 4021Got: 4022 0.84147098480789650665250232163029899962256306080 4023********************************************************************** 4024File "src/sage/libs/pari/pari_instance.pyx", line 187, in sage.libs.pari.pari_instance 4025Failed example: 4026 pari(1).sin(precision=20).sage() 4027Expected: 4028 0.841470984807896507 4029Got: 4030 0.841470985 4031********************************************************************** 40321 item had failures: 4033 2 of 55 in sage.libs.pari.pari_instance 4034 [164 tests, 2 failures, 0.33 s]
comment:17 Changed 6 years ago by
Right, we should only pick precisions which are between 33 and 64 mod 64.
comment:18 Changed 6 years ago by
Commit:  b70ec86f4f5aed9152123bb79fc5981e743cd19c → 3db32759c7a468f35ea578c16eca353b18663b54 

comment:19 Changed 6 years ago by
Status:  needs_work → needs_review 

comment:20 followup: 21 Changed 6 years ago by
Wouldn't it be simpler and clearer to replace 180→192 and 40→64?
comment:21 followup: 22 Changed 6 years ago by
Replying to defeo:
Wouldn't it be simpler and clearer to replace 180→192 and 40→64?
That would defeat the whole point of that explanation. I want to explain that precision in PARI is not the same as precision in Sage.
comment:22 followup: 24 Changed 6 years ago by
That would defeat the whole point of that explanation. I want to explain that precision in PARI is not the same as precision in Sage.
That was my guess, but I think in this case the docstring misses the point: unless he counts the digits by hand, the user won't realize that the output is printed with 192 bits of precision instead of 180. Maybe adding a parent(_)
to the docstring would help clarify? Something like this:
sage: pari(1).sin(precision=180).sage() 0.841470984807896506652502321630298999622563060798371065673 sage: x = pari(1).sin(precision=180) sage: x 0.841470984807897 sage: x.bitprecision() 192 sage: x.sage() 0.841470984807896506652502321630298999622563060798371065673 sage: parent(_) Real Field with 192 bits of precision
And maybe line 169 should print with 180 bits, to highlight the difference.
Also, in lines 182183, when you write "real precision", there is confusion btw real as in reality and real as in real number (frankly, after rereading for the 10th time that sentence, I'm not sure I understand it).
comment:23 followup: 25 Changed 6 years ago by
Continuing the previous example, I'm confused by this:
sage: x = pari(1).sin(precision=180) sage: x.precision() 5
what's the unit used by PARI?
comment:24 Changed 6 years ago by
Replying to defeo:
Also, in lines 182183, when you write "real precision", there is confusion btw real as in reality and real as in real number (frankly, after rereading for the 10th time that sentence, I'm not sure I understand it).
PARI uses the term "real precision" to mean precision of real numbers. I use it the same way.
comment:25 followup: 28 Changed 6 years ago by
Replying to defeo:
Continuing the previous example, I'm confused by this:
sage: x = pari(1).sin(precision=180) sage: x.precision() 5what's the unit used by PARI?
In this case, words (i.e. units of 32 or 64 bits)
comment:26 Changed 6 years ago by
Dependencies:  #20241 

comment:27 Changed 6 years ago by
Commit:  3db32759c7a468f35ea578c16eca353b18663b54 → a7099ae741973f0db3a767c8af1f57989ddacf15 

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

comment:28 followup: 30 Changed 6 years ago by
comment:29 Changed 6 years ago by
Status:  needs_review → positive_review 

comment:30 Changed 6 years ago by
Replying to defeo:
?? 5*32=160. How does that fit with 192 bits of precision?
It's actually (5  2) * 64 = 192
:
sage: from sage.libs.pari.pari_instance import prec_words_to_bits ....: prec_words_to_bits(5) 192
comment:31 Changed 6 years ago by
Branch:  u/jdemeyer/ticket/21703 → a7099ae741973f0db3a767c8af1f57989ddacf15 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
Merge commit '343131a9951dc842c462809b9366b13416382dfc'; commit '91c436752ba82d512efdf67e2890d5ecbe0b7234'; commit '74041f30d8de4c15055345f14340a4f443236ffa' into HEAD
Separate Sagespecific components from generic Cinterface in PariInstance
Simplify module_list
Cleanup imports; add __future__ statements
Fixed broken doctest in autogenerationy
Merge tag '7.4.rc0' into t/20241/ticket/20241
Interface PARI precision in bits