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: sage-7.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:

Status badges

Description (last modified by Jeroen Demeyer)

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 Jeroen Demeyer

Description: modified (diff)

comment:2 Changed 6 years ago by Jeroen Demeyer

Branch: u/jdemeyer/ticket/21703

comment:3 Changed 6 years ago by Jeroen Demeyer

Authors: Jeroen Demeyer
Commit: 313b785064eb21e16d55855c4c2216fa26ca8df0
Status: newneeds_review

New commits:

83cca0fMerge commit '343131a9951dc842c462809b9366b13416382dfc'; commit '91c436752ba82d512efdf67e2890d5ecbe0b7234'; commit '74041f30d8de4c15055345f14340a4f443236ffa' into HEAD
f35e562Separate Sage-specific components from generic C-interface in PariInstance
e363e4cSimplify module_list
968519cCleanup imports; add __future__ statements
e88d56cFixed broken doctest in auto-generationy
57e1cbaMerge tag '7.4.rc0' into t/20241/ticket/20241
313b785Interface PARI precision in bits

comment:4 Changed 6 years ago by Luca De Feo

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:5 Changed 6 years ago by Luca De Feo

Or maybe in __init__.py, so that sage.libs.pari? shows it.

comment:6 Changed 6 years ago by Luca De Feo

set_real_precision() returns the old precision. set_real_precision_bits() doesn't return anything. Couldn't we be more consistent here?

comment:7 in reply to:  6 ; Changed 6 years ago by Jeroen Demeyer

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 in reply to:  7 ; Changed 6 years ago by Luca De Feo

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 call get_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 in reply to:  8 Changed 6 years ago by Jeroen Demeyer

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 Changed 6 years ago by Luca De Feo

Sure, that's no big deal.

What about the docstrings?

comment:11 in reply to:  10 Changed 6 years ago by Jeroen Demeyer

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 git

Commit: 313b785064eb21e16d55855c4c2216fa26ca8df0b70ec86f4f5aed9152123bb79fc5981e743cd19c

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

f80485fRemove obsolete comment about stack increasing
b70ec86Add reference to precision guide in PariInstance.__init__

comment:13 Changed 6 years ago by Luca De Feo

Reviewers: Luca De Feo
Status: needs_reviewpositive_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 Luca De Feo

Weird, the startup modules plugin is failing. Presumably not related to this?

comment:15 Changed 6 years ago by Jeroen Demeyer

The startup modules "failure" is from #20241.

comment:16 Changed 6 years ago by Volker Braun

Status: positive_reviewneeds_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 Jeroen Demeyer

Right, we should only pick precisions which are between 33 and 64 mod 64.

comment:18 Changed 6 years ago by git

Commit: b70ec86f4f5aed9152123bb79fc5981e743cd19c3db32759c7a468f35ea578c16eca353b18663b54

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

b4bc24fMerge tag '7.5.beta1' into t/21703/ticket/21703
3db3275Only use precisions which are in [33,64] mod 64

comment:19 Changed 6 years ago by Jeroen Demeyer

Status: needs_workneeds_review

comment:20 Changed 6 years ago by Luca De Feo

Wouldn't it be simpler and clearer to replace 180→192 and 40→64?

comment:21 in reply to:  20 ; Changed 6 years ago by Jeroen Demeyer

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 in reply to:  21 ; Changed 6 years ago by Luca De Feo

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 182-183, when you write "real precision", there is confusion btw real as in reality and real as in real number (frankly, after re-reading for the 10th time that sentence, I'm not sure I understand it).

comment:23 Changed 6 years ago by Luca De Feo

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 in reply to:  22 Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

Also, in lines 182-183, when you write "real precision", there is confusion btw real as in reality and real as in real number (frankly, after re-reading 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 in reply to:  23 ; Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

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?

In this case, words (i.e. units of 32 or 64 bits)

comment:26 Changed 6 years ago by Jeroen Demeyer

Dependencies: #20241

comment:27 Changed 6 years ago by git

Commit: 3db32759c7a468f35ea578c16eca353b18663b54a7099ae741973f0db3a767c8af1f57989ddacf15

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

a7099aeImprove documentation

comment:28 in reply to:  25 ; Changed 6 years ago by Luca De Feo

Replying to jdemeyer:

Replying to defeo:

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?

In this case, words (i.e. units of 32 or 64 bits)

?? 5*32=160. How does that fit with 192 bits of precision?

comment:29 Changed 6 years ago by Luca De Feo

Status: needs_reviewpositive_review

comment:30 in reply to:  28 Changed 6 years ago by Jeroen Demeyer

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 Volker Braun

Branch: u/jdemeyer/ticket/21703a7099ae741973f0db3a767c8af1f57989ddacf15
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.