Opened 11 years ago
Last modified 9 years ago
#11868 closed defect
PARI library interface broken by design — at Version 13
Reported by: | Jeroen Demeyer | Owned by: | William Stein |
---|---|---|---|
Priority: | major | Milestone: | sage-5.13 |
Component: | interfaces | Keywords: | t0GEN t1GEN gen |
Cc: | Merged in: | ||
Authors: | Peter Bruin | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The t0GEN()
, t1GEN()
,... system in the PARI interface is broken by design. Apply demonstrate_11868.patch to see the bug in action.
The problem is that the conversion of b
(Python object) to t1
(PARI GEN) uses the PARI stack and therefore it clobbers the previously computed t0
.
There is no urgent need to fix this, as the bug doesn't seem to occur in practice (it was discovered when working on #9334). It only occurs when:
- Calling a PARI function with at least 2 arguments besides
self
. - Apart from the first non-
self
argument, there should be another argument which is a "complicated" Python type (e.g. a number field element).
As work-around, it is easy to defeat condition 2 by converting arguments to pari before calling the function.
This problem can be solved by using only local variables instead of t0GEN
etc.
Apply: trac_11868-t0GEN.patch, trac_11868-remove_workarounds.patch
Change History (16)
comment:1 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 9 years ago by
Authors: | → Peter Bruin |
---|---|
Description: | modified (diff) |
Milestone: | → sage-5.13 |
Status: | new → needs_review |
comment:5 Changed 9 years ago by
The patch contains (slightly) more changes than strictly necessary; this is to make life a bit easier for #15185.
comment:6 Changed 9 years ago by
Is it not possible to keep get_nf()
as a function returning a GEN
without copying?
comment:7 Changed 9 years ago by
I agree with the general approach, but have to check the many details...
comment:8 follow-up: 12 Changed 9 years ago by
Reviewers: | → Jeroen Demeyer |
---|---|
Status: | needs_review → needs_work |
In the PARI sources, there are various workarounds for #11868, could you undo these workarounds:
devel/sage/sage/schemes/elliptic_curves/ell_point.py: We need to explicitly call ``pari()`` because of :trac:`11868`:: devel/sage/sage/rings/number_field/number_field.py: # to work around Trac #11868 -- Jeroen Demeyer devel/sage/sage/rings/number_field/number_field.py: # to work around Trac #11868 -- Jeroen Demeyer devel/sage/sage/libs/pari/gen.pyx: sage: pari(K).nfhilbert(t, t+2) # not tested, known bug #11868
comment:9 Changed 9 years ago by
Maybe rename get_nf()
as _get_nf()
to emphazise that it's a private internal function?
comment:10 Changed 9 years ago by
One important "detail": it seems this ticket causes a lot more copying of data (because of the gcopy()
in cdef GEN toGEN
. That's bad.
comment:11 Changed 9 years ago by
Maybe the right thing to do is to have things like t0
of type gen
instead of GEN
and then using t0.g
(like we already often do for self
).
comment:12 Changed 9 years ago by
Changed 9 years ago by
Attachment: | trac_11868-remove_workarounds.patch added |
---|
remove workarounds for bug related to global GEN variables
comment:13 Changed 9 years ago by
Description: | modified (diff) |
---|
The
t0GEN()
,t1GEN()
,... system in the PARI interface is broken by design. Apply demonstrate_11868.patch to see the bug in action.