Opened 6 years ago

Closed 6 years ago

#22193 closed enhancement (fixed)

Port cypari2 documentation as a standalone doctests

Reported by: Vincent Delecroix Owned by:
Priority: major Milestone: sage-7.6
Component: packages: standard Keywords:
Cc: Luca De Feo, Jeroen Demeyer Merged in:
Authors: Vincent Delecroix, Luca De Feo Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 128030a (Commits, GitHub, GitLab) Commit: 128030ad25fc7eb86c3995cab3a399d672c5e6a4
Dependencies: #22319 Stopgaps:

Status badges

Description (last modified by Vincent Delecroix)

Since cypari2 is aimed to become an external module we should move the doctest of its documentation in the standalone file src/sage/libs/pari/tests.py.

Attachments (1)

pari_doctests.py (1.5 KB) - added by Vincent Delecroix 6 years ago.

Download all attachments as: .zip

Change History (70)

comment:1 Changed 6 years ago by Jeroen Demeyer

We already have src/sage/libs/pari/tests.py. That would be a good place to put those tests.

comment:2 Changed 6 years ago by Vincent Delecroix

Description: modified (diff)

comment:3 Changed 6 years ago by Vincent Delecroix

Authors: Vincent Delecroix

doing that...

Changed 6 years ago by Vincent Delecroix

Attachment: pari_doctests.py added

comment:4 Changed 6 years ago by Vincent Delecroix

I joined the script I used to generate the files.

comment:5 Changed 6 years ago by Vincent Delecroix

Branch: u/vdelecroix/22193
Commit: 3f211e45626df9f6338542decb4ef0df484fd7ff
Status: newneeds_review

New commits:

3f211e422193: copy doctests from cypari2

comment:6 Changed 6 years ago by Jeroen Demeyer

I don't really like that this is automatic. I think we should manually figure out which doctests should be kept in cypari2 and which ones should be moved to Sage.

comment:7 Changed 6 years ago by Jeroen Demeyer

I think that the tests in convert.pyx don't really involve Sage at all (except for the preparser, so 2^64 should be changed to 2**64 for example).

comment:8 Changed 6 years ago by Luca De Feo

What's the status of this? Anyone working on it?

comment:9 Changed 6 years ago by Luca De Feo

Branch: u/vdelecroix/22193u/defeo/22193

comment:10 Changed 6 years ago by Luca De Feo

Authors: Vincent DelecroixVincent Delecroix, Luca De Feo
Commit: 3f211e45626df9f6338542decb4ef0df484fd7ffb6f22af9012c3f0e67511fabccf65e519ab43050
Dependencies: #22221 #22222 #22291
Status: needs_reviewneeds_work

It's been extremely painful, but I've manually gone through all docstrings. As Jeroen suspected, very few of them were Sage-specific, but most of them needed to be de-Saged.

I moved the few Sage-specific tests to test.py, and also duplicated there some tests that could have been done entirely in CyPari2, but that used to activate some code-paths in Sage that wouldn't be activated otherwise.

There's two notable bits of docstring I haven't touched yet:

  1. The "Guide to real precision" in the module docstring of pari_instance.pyx. This is very Sage-specific, and it's more than just a testsuite. Where shall we move it?
  1. The Gen.sage(), which of course has no place in CyPari?. Did we decide anything on subclassing Gen, though?

For the rest, I hopefully changed all ^ to ** at the right spots, and removed all dependencies on Sage (except for the sage: prompt and the import statements). If I forgot something, we will notice anyway when setting up tests outside of Sage. I also fear that some doctests may fail in Python3, but that should also be easy to fix.

In my doctest trip, I saw a lot of very archaic code, esp. in the number field and the elliptic curve methods. For example nfbasis has a very weird docstring, and takes a flag parameter that seems to be undocumented in PARI.


New commits:

41d47f7Fix dependency on PARI headers
1755068Merge commit '41d47f7a14d9844c3a030c234a2c5d4a8971944f' of trac.sagemath.org:sage into t/22193/port_cypary2_doctests
98869b4Remove _pari_instance global
c9c93dcMerge commit '98869b4f83de894281bf164016d7ef9a2f4b3227' of trac.sagemath.org:sage into t/22193/port_cypary2_doctests
b5f74b1Use longintrepr.pxd from Cython
c6d3236Merge commit 'b5f74b1a88203cbc1768e47a2ae2037a1b6f18d6' of trac.sagemath.org:sage into t/22193/port_cypary2_doctests
b6f22afMade docstrings in libs/cypari2 mostyl Sage-independent

comment:11 Changed 6 years ago by git

Commit: b6f22af9012c3f0e67511fabccf65e519ab4305052e9f8c03f97b53406c869c521264399a5ccd7cb

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

52e9f8cRemoved Sage-specific bits from guide to real precision

comment:12 Changed 6 years ago by Luca De Feo

Status: needs_workneeds_review

comment:13 Changed 6 years ago by Jeroen Demeyer

Just had a quick look and it looks good on first sight. I need to check a bit better in order to give positive_review though.

Does the documentation from src/sage/libs/pari/__init__.py actually appear anywhere?

comment:14 in reply to:  13 ; Changed 6 years ago by Luca De Feo

Does the documentation from src/sage/libs/pari/__init__.py actually appear anywhere?

I don't think so, but all documentation from cypari2 seems to have disappeared from the reference manual anyway. I thought that was a deliberate choice?

comment:15 Changed 6 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

Please rebase to 7.6.beta3

comment:16 in reply to:  14 Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

all documentation from cypari2 seems to have disappeared from the reference manual anyway.

The code for src/sage/libs/cypari2 will be removed from Sage anyway, so I don't consider it a problem that the doc is not in Sage anymore.

However, the doc for src/sage/libs/pari should remain in Sage. It is currently not.

comment:17 Changed 6 years ago by Jeroen Demeyer

Branch: u/defeo/22193u/jdemeyer/22193

comment:18 Changed 6 years ago by Jeroen Demeyer

Commit: 52e9f8c03f97b53406c869c521264399a5ccd7cb3cb08162bfbc1d5b64f8899bd455907e6ddec937

Rebased (and fixed a typo in the commit message).


New commits:

8608c94Made docstrings in libs/cypari2 mostly Sage-independent
3cb0816Removed Sage-specific bits from guide to real precision

comment:19 Changed 6 years ago by Jeroen Demeyer

Dependencies: #22221 #22222 #22291
Reviewers: Jeroen Demeyer

I'm having a look at the documentation right now.

comment:20 Changed 6 years ago by git

Commit: 3cb08162bfbc1d5b64f8899bd455907e6ddec937db744ba0f606e1fda2fa7cac94b64baafaaf1aa5

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

db744baAdd sage.libs.pari to the documentation

comment:21 Changed 6 years ago by Jeroen Demeyer

Status: needs_workneeds_review

comment:22 Changed 6 years ago by Luca De Feo

Status: needs_reviewpositive_review

Thanks for the rebase.

We could improve the documentation of sage.libs.pari, but that's probably best kept for another ticket.

comment:23 Changed 6 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

Some details...

  1. You forgot to remove one int() here:
    sage: int(2) ** pari(-5)
    
  1. Why did you remove this?
    sage: bnr = pari("K = bnfinit(x^4 - 4*x^2 + 1); bnrinit(K, 2*x)")
    sage: bnr.nf_get_pol()
    
  1. This sentence is no longer relevant:
    However, beware that ``pari(L)`` returns an absolute number field::
    
  1. This is a significant change:
    -            sage: two = RealField(256)(2)._pari_()
    +            sage: two = pari(2)
    

Instead, you could use

two = pari("2.0000000000000000000000000000000000000000000000000000000000000000000000000000")
  1. This is confusing since the 5/3 is not the fraction 5/3 in Python!
    sage: pari([-1, 5/3, -8.0]).vecmin()
    

You could keep the original test with quotes: pari("[1, -5/3, 8.0]").vecmin()

comment:24 in reply to:  23 ; Changed 6 years ago by Luca De Feo

  1. You forgot to remove one int() here:
    sage: int(2) ** pari(-5)
    

This breaks in Sage, unless we positive review #22321. But I wanted to explore that ticket a bit more before setting it to needs review.

On the other hand, it doesn't hurt Python.

  1. Why did you remove this?
    sage: bnr = pari("K = bnfinit(x^4 - 4*x^2 + 1); bnrinit(K, 2*x)")
    sage: bnr.nf_get_pol()
    

I did not remove it. I changed it to

sage: x = pari('x')                                                                                                         
sage: K = (x**4 - 4*x**2 + 1).nfinit()
sage: K.nf_get_pol()                                                                                                        

which seems to be functionally equivalent and more concise. However, you know PARI better than me. If you think that's more appropriate, I can change it to

sage: x = pari('x')                                                                                                         
sage: K = (x**4 - 4*x**2 + 1).bnfinit().bnrinit(2*x)                                                                        
sage: K.nf_get_pol()                                                                                                        

comment:25 Changed 6 years ago by Luca De Feo

Branch: u/jdemeyer/22193u/defeo/22193

comment:26 Changed 6 years ago by Luca De Feo

Commit: db744ba0f606e1fda2fa7cac94b64baafaaf1aa53e4bc1c84add1fc78e8951ae3d531cc59e033c42

fixed the rest


New commits:

3e4bc1cRestored some doctests

comment:27 Changed 6 years ago by git

Commit: 3e4bc1c84add1fc78e8951ae3d531cc59e033c424efdc2d774abfb09d9cb7d8ecee168091356bc5b

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

0348e00Implement __index__ for PARI Gen
50593d6Minor fixes to __index__
83965bdMerge branch 'u/jdemeyer/implement___index___for_pari_gen' of git://trac.sagemath.org/sage into t/22193/22193
4efdc2dRemoved one unnecessary conversion to int in doctest

comment:28 Changed 6 years ago by Luca De Feo

Dependencies: #22319

Replying to defeo:

  1. You forgot to remove one int() here:
    sage: int(2) ** pari(-5)
    

This breaks in Sage, unless we positive review #22321.

Wait, no, I got confused: it just needs #22319, which is positive reviewed. Fixed now.

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

Replying to defeo:

... which seems to be functionally equivalent

But the bnrinit() call is gone, so it's clearly a different test.

Why not keep the original test? It should work as written in CyPari2.

comment:30 in reply to:  29 ; Changed 6 years ago by Luca De Feo

Replying to jdemeyer:

Replying to defeo:

... which seems to be functionally equivalent

But the bnrinit() call is gone, so it's clearly a different test.

Why not keep the original test? It should work as written in CyPari2.

Because examples in the documentation should show the proper way to do things, not the ugly one.

I admit I do not understand the reason for the call to bnrinit(), maybe you can elucidate.

We can put the old test in TESTS::, however, if you wish to keep it.

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

Replying to defeo:

I admit I do not understand the reason for the call to bnrinit(), maybe you can elucidate.

It shows that nf_get_pol() works when given a bnr (the output of bnrinit()) as input. So it is an important test which really should not be removed.

comment:32 Changed 6 years ago by git

Commit: 4efdc2d774abfb09d9cb7d8ecee168091356bc5b96ddcd3616f7c18b757a34af4a15241b0696439d

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

96ddcd3Restored old example with bnr

comment:33 Changed 6 years ago by Luca De Feo

Status: needs_workneeds_review

what about this?

comment:34 Changed 6 years ago by Jeroen Demeyer

Status: needs_reviewpositive_review

comment:35 Changed 6 years ago by Volker Braun

Branch: u/defeo/2219396ddcd3616f7c18b757a34af4a15241b0696439d
Resolution: fixed
Status: positive_reviewclosed

comment:36 Changed 6 years ago by Volker Braun

Commit: 96ddcd3616f7c18b757a34af4a15241b0696439d
Resolution: fixed
Status: closednew

fails on 32-bit:

sage -t --long src/sage/libs/cypari2/pari_instance.pyx
3569**********************************************************************
3570File "src/sage/libs/cypari2/pari_instance.pyx", line 112, in sage.libs.cypari2.pari_instance
3571Failed example:
3572    pari(float(1.0)).sin(precision=180) - pari(1).sin(precision=180)
3573Expected:
3574    5.42101086242752 E-20
3575Got:
3576    5.42101086 E-20
3577**********************************************************************
35781 item had failures:
3579   1 of  31 in sage.libs.cypari2.pari_instance
3580    [151 tests, 1 failure, 0.33 s]

comment:37 Changed 6 years ago by Luca De Feo

Branch: 96ddcd3616f7c18b757a34af4a15241b0696439du/defeo/96ddcd3616f7c18b757a34af4a15241b0696439d

comment:38 Changed 6 years ago by Luca De Feo

Commit: 27251c31be002d282b0b10d9cdef92d07ee1d9c1
Status: newneeds_review

New commits:

8608c94Made docstrings in libs/cypari2 mostly Sage-independent
3cb0816Removed Sage-specific bits from guide to real precision
db744baAdd sage.libs.pari to the documentation
3e4bc1cRestored some doctests
83965bdMerge branch 'u/jdemeyer/implement___index___for_pari_gen' of git://trac.sagemath.org/sage into t/22193/22193
4efdc2dRemoved one unnecessary conversion to int in doctest
96ddcd3Restored old example with bnr
27251c3Fixed doctests failing on 32 bits, and improved docs

comment:39 Changed 6 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

I think it would be better to keep the spirit of the original examples instead of inventing a pointless example like pari(float(1.0)).sin(precision=180) - pari(1).sin(precision=180) == 0. Remember that these doctests are meant as documentation and I don't see what you are documenting there...

comment:40 in reply to:  39 ; Changed 6 years ago by Luca De Feo

Replying to jdemeyer:

I think it would be better to keep the spirit of the original examples instead of inventing a pointless example like pari(float(1.0)).sin(precision=180) - pari(1).sin(precision=180) == 0. Remember that these doctests are meant as documentation and I don't see what you are documenting there...

The original documentation said

In the third case, the precision is determined only by the inexact
inputs and the ``precision`` argument is ignored::
                                                  
    sage: pari(1.0).sin(precision=180).sage()
    0.841470984807896507
    sage: pari(1.0).sin(precision=40).sage()
    0.841470984807896507
    sage: pari(RealField(100).one()).sin().sage()
    0.84147098480789650665250232163029899962

which has no equivalent in CyPari?. Would you find this clearer?

    sage: d = pari(float(1.0)).sin(precision=180); d
    0.8414709848078965067
    sage: d.bitprecision()                                 
    64

However this test is also architecture dependent.

Last edited 6 years ago by Luca De Feo (previous) (diff)

comment:41 in reply to:  40 Changed 6 years ago by Luca De Feo

@jdemeyer ping?

Replying to defeo:

Replying to jdemeyer: Would you find this clearer?

    sage: d = pari(float(1.0)).sin(precision=180); d
    0.8414709848078965067
    sage: d.bitprecision()                                 
    64

However this test is also architecture dependent.

comment:42 in reply to:  40 Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

which has no equivalent in CyPari?.

I disagree with this.

There are plenty of ways to create high-precision numbers in CyPari?:

sage: n = pari("1.0000000000000000000000000000"); n.bitprecision()
128
sage: n = pari("localbitprec(100); 1.0"); n.bitprecision()
128
sage: n = pari(1.0).bitprecision(100); n.bitprecision()
128

Personally, I would prefer the third way.

So I think that you can mostly keep the old doctests, just replacing pari(RealField(100)(1.0)) by pari(1.0).bitprecision(100).

comment:43 Changed 6 years ago by Jeroen Demeyer

Looking at the documentation of precision more carefully, I think you should actually restore larger parts of the docs. For example, the first part of "Internal representation and conversion between Sage and PARI" and all of "Output precision for printing" should be kept for CyPari2.

comment:44 in reply to:  43 ; Changed 6 years ago by Luca De Feo

Replying to jdemeyer:

For example, the first part of "Internal representation and conversion between Sage and PARI"

There's pretty much nothing that can be salvaged from this section: precision in PARI is in multiples of 32 or 64. That's it.

and all of "Output precision for printing" should be kept for CyPari2.

I think this section is completely misleading, and the more I try to understand, the less I get it. It talks about "printing" precision, set_real_precision seems to affect more than printing:

sage: pari.set_real_precision_bits(1000)
sage: a = pari('Pi')
sage: 
sage: pari.set_real_precision_bits(10)
sage: b = pari('Pi')
sage: a - b
-2.17 E-19
sage: pari.set_real_precision_bits(1000)
sage: a - b
-2.168404344971008868 E-19
sage: a - b.bitprecision(1000)
-5.016557612668332023557327080330757013833665769218359371379100137196517465788293201785191348671769335290615539044941776827464059187151888254971589729806147889444035537705104506961803557118902433406655387152435176621321683472879809085435143307653965138954567335178660639273975085872627541300 E-20

I think this whole part needs to be rewritten, not translated from the old docs. Problem is, I'm not sure I understand what it does.

comment:45 in reply to:  44 Changed 6 years ago by Jeroen Demeyer

Can you explain me what you don't understand?

I could certainly try to improve the docs, but then I need to know what's wrong with the current docs.

comment:46 in reply to:  44 Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

There's pretty much nothing that can be salvaged from this section: precision in PARI is in multiples of 32 or 64. That's it.

...but that is a very important fact to mention which is certainly more than "pretty much nothing".

comment:47 in reply to:  44 Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

I think this section is completely misleading, and the more I try to understand, the less I get it. It talks about "printing" precision, set_real_precision seems to affect more than printing:

I hope that "completely misleading" is an exaggeration. The sentence "The maximum number of digits when printing a PARI real can be set using the methods Pari.set_real_precision_bits or Pari.set_real_precision" is correct but it's true that set_real_precision_bits affects more than just printing. It also affects precision for numbers parsed using the string interface, which is documented in the "Input precision for function calls" section. So I wonder where your confusion comes from.

comment:48 Changed 6 years ago by Luca De Feo

Sorry, I shouldn't be doing so many things in parallel. I'll try to frame my doubts better.

For one, I do not understand this:

sage: pi = pari('Pi')
sage: a = pari(float(1.0))
sage: pari.set_real_precision(500)
15
sage: pi
3.141592653589793239
sage: a
1.0000000000000000000
sage: pi.bitprecision(1000)
3.1415926535897932385128089594061862044327426701784133911132812500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
sage: a.bitprecision(1000)
1.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

How does PARI decide what bits to add when calling bitprecision()? Why does Pi have junk digits before all the zeros, while 1.0 gets only added zeros?

comment:49 Changed 6 years ago by Luca De Feo

Then, there's the fact that I expect (too optimistically) Pi to be a symbolic quantity, i.e. that incrementing precision (with pi.bitprecision(1000)) would compute correctly the new digits instead of adding random junk.

That's probably asking too much from PARI, but I got bad habits from Sage :)

The same behavior from pari.pi() surprises me much less: when I see a function call, I understand that's a function supposed to compute π up to a fixed precision.

Anyway, not much we can do here: that's the way PARI it is.

comment:50 Changed 6 years ago by Luca De Feo

And, finally, when I read

The maximum number of digits when printing a PARI real can be set using the methods :meth:Pari.set_real_precision_bits or :meth:Pari.set_real_precision.

I understand that's the SOLE role of set_real_precision: controlling output precision. Then, when I go further down, I discover that it also controls input precision for strings.

I would find it clearer, if it was stated right away that set_real_precision controls input and output precision for strings.

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

Replying to defeo:

Sorry, I shouldn't be doing so many things in parallel. I'll try to frame my doubts better.

For one, I do not understand this:

sage: pi = pari('Pi')
sage: a = pari(float(1.0))
sage: pari.set_real_precision(500)
15
sage: pi
3.141592653589793239
sage: a
1.0000000000000000000
sage: pi.bitprecision(1000)
3.1415926535897932385128089594061862044327426701784133911132812500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
sage: a.bitprecision(1000)
1.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

How does PARI decide what bits to add when calling bitprecision()?

It just adds zero bits.

Why does Pi have junk digits before all the zeros, while 1.0 gets only added zeros?

You should not make any conclusions from the decimal representation of the number, in binary it's just zeros.

Replying to defeo:

Then, there's the fact that I expect (too optimistically) Pi to be a symbolic quantity, i.e. that incrementing precision (with pi.bitprecision(1000)) would compute correctly the new digits instead of adding random junk.

PARI doesn't really do symbolic computations. Pi does a computation returning a floating-point approximation of π.

The same behavior from pari.pi() surprises me much less: when I see a function call, I understand that's a function supposed to compute π up to a fixed precision.

Actually, Pi is a function call. This works:

gp> Pi()
%1 = 3.1415926535897932384626433832795028842

It's just that the GP parser turns Pi into Pi() to improve user-friendliness.

comment:52 in reply to:  51 Changed 6 years ago by Luca De Feo

Why does Pi have junk digits before all the zeros, while 1.0 gets only added zeros?

You should not make any conclusions from the decimal representation of the number, in binary it's just zeros.

Right. So I think

n = pari(1.0).bitprecision(100)

is giving bad advice to the user: it only gets the intended value by chance, and the doctstring of bitprecision does nothing to help the user guess what the result would be.

It's just that the GP parser turns Pi into Pi() to improve user-friendliness.

I call this user-hateness.

comment:53 Changed 6 years ago by Jeroen Demeyer

Feel free to complain to the PARI developers for anything that you don't like...

comment:54 Changed 6 years ago by Jeroen Demeyer

Luca, are you going to work on this, or should I?

comment:55 Changed 6 years ago by Luca De Feo

Sorry, I was busy teaching. I was meaning to finalize this, but if I don't make it by today, it will have to wait until Fri.

Of course, if you have time to donate to this, feel free to jump in.

comment:56 Changed 6 years ago by Jeroen Demeyer

OK, I will try to have a look today.

comment:57 Changed 6 years ago by Jeroen Demeyer

Branch: u/defeo/96ddcd3616f7c18b757a34af4a15241b0696439du/jdemeyer/96ddcd3616f7c18b757a34af4a15241b0696439d

comment:58 Changed 6 years ago by git

Commit: 27251c31be002d282b0b10d9cdef92d07ee1d9c1fb79fd9275b295ac10a3d57f7acff997879f41c6

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

fb79fd9Update documentation about precision

comment:59 Changed 6 years ago by Jeroen Demeyer

Status: needs_workneeds_review

comment:60 Changed 6 years ago by Jeroen Demeyer

I hope this helps...

comment:61 Changed 6 years ago by Luca De Feo

Status: needs_reviewpositive_review

Sorry for the delay. It is clearer now, thanks for the patch.

Are you sure the doctests pass on 32-bits? This, for example:

sage: c = pari(1).sin(); c
0.841470984807897
sage: c.bitprecision()
64

comment:62 in reply to:  61 ; Changed 6 years ago by Luca De Feo

Are you sure the doctests pass on 32-bits? This, for example:

sage: c = pari(1).sin(); c
0.841470984807897
sage: c.bitprecision()
64

Ok, went again through the examples, and this example is obviously not a problem. The only problematic tests are those that do pari(1.0): that's not arch dependent in Sage (because 1.0 is a real), but it is in Python.

Anyway, they are ok for now. We shall see when we bring this to cypari.

comment:63 in reply to:  62 Changed 6 years ago by Jeroen Demeyer

Replying to defeo:

The only problematic tests are those that do pari(1.0): that's not arch dependent in Sage (because 1.0 is a real), but it is in Python.

No, Python floats are the same on 32-bit and 64-bit. It's true that certain computations involving floats might give different results. But for a conversion, the results should be the same on 32-bit and 64-bit.

comment:64 Changed 6 years ago by Volker Braun

Status: positive_reviewneeds_work
sage -t --long src/sage/libs/cypari2/pari_instance.pyx
**********************************************************************
File "src/sage/libs/cypari2/pari_instance.pyx", line 91, in sage.libs.cypari2.pari_instance
Failed example:
    pari("Pi") - p
Expected:
    1.22514845490862 E-16
Got:
    1.225148455 E-16
**********************************************************************
File "src/sage/libs/cypari2/pari_instance.pyx", line 94, in sage.libs.cypari2.pari_instance
Failed example:
    pari("Pi") - p
Expected:
    1.22514845490862 E-16
Got:
    1.225148455 E-16
**********************************************************************
1 item had failures:
   2 of  54 in sage.libs.cypari2.pari_instance
    [174 tests, 2 failures, 0.22 s]

comment:65 Changed 6 years ago by Jeroen Demeyer

Sorry for that.

comment:66 Changed 6 years ago by git

Commit: fb79fd9275b295ac10a3d57f7acff997879f41c6128030ad25fc7eb86c3995cab3a399d672c5e6a4

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

128030aLess digits are shown on 32-bit

comment:67 Changed 6 years ago by Jeroen Demeyer

Status: needs_workneeds_review

comment:68 Changed 6 years ago by Jeroen Demeyer

Status: needs_reviewpositive_review

This fixes the 32-bit tests.

comment:69 Changed 6 years ago by Volker Braun

Branch: u/jdemeyer/96ddcd3616f7c18b757a34af4a15241b0696439d128030ad25fc7eb86c3995cab3a399d672c5e6a4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.