Opened 5 years ago
Closed 5 years ago
#23957 closed defect (fixed)
gmpy2 causes doctest failures
Reported by:  jdemeyer  Owned by:  

Priority:  blocker  Milestone:  sage8.1 
Component:  packages: optional  Keywords:  
Cc:  vdelecroix, mderickx  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Vincent Delecroix, Maarten Derickx 
Report Upstream:  N/A  Work issues:  
Branch:  fea3848 (Commits, GitHub, GitLab)  Commit:  fea3848d0d397a7abbc196b3de93df48a2b0fbc3 
Dependencies:  Stopgaps: 
Description
The current version of gmpy2
has issues, in particular doctest failures like
sage t src/sage/ext/memory.pyx ********************************************************************** File "src/sage/ext/memory.pyx", line 9, in sage.ext.memory Failed example: 2^(2^632) Expected: Traceback (most recent call last): ... MemoryError: failed to allocate 1152921504606847008 bytes Got: Fatal Python error: Insufficient memory Traceback (most recent call last): File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 515, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 885, in compile_and_execute exec(compiled, globs) File "<doctest sage.ext.memory[0]>", line 1, in <module> Integer(2)**(Integer(2)**Integer(63)Integer(2)) File "sage/rings/integer.pyx", line 2067, in sage.rings.integer.Integer.__pow__ (build/cythonized/sage/rings/integer.c:14173) sig_on() RuntimeError: Aborted **********************************************************************
Change History (15)
comment:1 Changed 5 years ago by
 Cc mderickx added
 Component changed from packages: experimental to packages: optional
 Priority changed from major to blocker
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
 Branch set to u/jdemeyer/gmpy2_causes_doctest_failures
comment:4 Changed 5 years ago by
 Commit set to fea3848d0d397a7abbc196b3de93df48a2b0fbc3
 Status changed from new to needs_review
New commits:
fea3848  Do not call mp_set_memory_functions() in gmpy2

comment:5 followup: ↓ 6 Changed 5 years ago by
Is this independent of whether gmp or mpir is used?
comment:6 in reply to: ↑ 5 Changed 5 years ago by
Replying to mderickx:
Is this independent of whether gmp or mpir is used?
What do you mean with "this"?
comment:7 followup: ↓ 8 Changed 5 years ago by
I mean the doctest failure, is your install using gmp or mpir. Or is this something that does not matter?
comment:8 in reply to: ↑ 7 Changed 5 years ago by
Replying to mderickx:
I mean the doctest failure, is your install using gmp or mpir.
I'm using mpir, but the failure is caused by gmpy2. So I still don't really understand your question.
comment:9 Changed 5 years ago by
Luckily, this is also fixed in gmpy2 version 2.X
comment:10 Changed 5 years ago by
Well gmpy2 is a python interface that can use both mpir or gmp as back end. And we already know that 2^(2^632)
causes different errors depending on wether one uses gmp or mpir. I dont know much about the inner workings of gmpy2 so maybe my question does not make sense to you because you know more about the inner workings of gmpy2 then I do, but with my current knowledge I wouldn't be surprised if memory management works differently depending whether mpir or gmp is used as backend, and this therefore deserves testing both with mpir and gmp.
comment:11 Changed 5 years ago by
I don't see the point of testing this on GMP (though it would not hurt). gmpy2
was "wrongly" setting a custom allocation scheme in GMP (or MPIR). Moreover the fix proposed is the way it is in development versions of gmpy2.
comment:12 Changed 5 years ago by
Note that without this ticked, but with gmp and gmpy2 there is just the expected gmp error:
btnacb021:sage_gmp mderickx$ ./sage ┌────────────────────────────────────────────────────────────────────┐ │ SageMath version 8.1.beta7, Release Date: 20171003 │ │ Type "notebook()" for the browserbased notebook interface. │ │ Type "help()" for help. │ └────────────────────────────────────────────────────────────────────┘ ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ ┃ Warning: this is a prerelease version, and it may be unstable. ┃ ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ Forcing sagelocation, probably because a new package was installed. Cleaning up, do not interrupt this. Done cleaning. sage: 2^(2^632) gmp: overflow in mpz type  RuntimeError Traceback (most recent call last) <ipythoninput1b138756ea9ad> in <module>() > 1 Integer(2)**(Integer(2)**Integer(63)Integer(2)) /Applications/sage_gmp/src/sage/rings/integer.pyx in sage.rings.integer.Integer.__pow__ (build/cythonized/sage/rings/integer.c:14250)() 2065 cdef Integer x = PY_NEW(Integer) 2066 > 2067 sig_on() 2068 mpz_pow_ui(x.value, (<Integer>self).value, nn if nn > 0 else nn) 2069 sig_off() RuntimeError: Aborted sage: optional_packages()[0] ['ccache', 'gmp', 'gmpy2', 'python2']
I do get the error reported in this ticket with just mpir. So there is difference between using gmp and mpir. I am now testing everything with this ticket applied
comment:13 Changed 5 years ago by
Ok this ticket passes the 2^(2^632)
test both with mpir and gmp for my on OS X 0.12.4 . I do not understand the exact change and its ramification, but it seems that Vincent does so I leave it up to him to give positive review.
comment:14 Changed 5 years ago by
 Reviewers set to Vincent Delecroix, Maarten Derickx
 Status changed from needs_review to positive_review
GMP/MPIR allows for custom memory allocation function (in replacement for malloc
/realloc
/free
). gmpy2
was changing them by default which is not a reasonable policy.
comment:15 Changed 5 years ago by
 Branch changed from u/jdemeyer/gmpy2_causes_doctest_failures to fea3848d0d397a7abbc196b3de93df48a2b0fbc3
 Resolution set to fixed
 Status changed from positive_review to closed
Why is gmpy2 involved here!?