Opened 7 years ago
Closed 7 years ago
#18052 closed enhancement (fixed)
Wrap Python functions in a PARI t_CLOSURE
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.6 |
Component: | interfaces | Keywords: | |
Cc: | pbruin | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Peter Bruin |
Report Upstream: | N/A | Work issues: | |
Branch: | c14da15 (Commits, GitHub, GitLab) | Commit: | c14da15b4cd3c42e3d380edfc685d488ddfcd884 |
Dependencies: | Stopgaps: |
Description (last modified by )
To improve support for PARI functions taking closures, it would be very nice if we could wrap an arbitrary Python function in a t_CLOSURE
.
This can be done using the PARI library functions install()
and snm_closure()
.
Change History (13)
comment:1 Changed 7 years ago by
- Description modified (diff)
- Summary changed from Wrap Python functions in a PARI entree to Wrap Python functions in a PARI entree and t_CLOSURE
comment:2 Changed 7 years ago by
- Branch set to u/jdemeyer/wrap_python_functions_in_a_pari_entree_and_t_closure
comment:3 Changed 7 years ago by
- Commit set to 1cd2a3df5b993c9593a75d94a8dcd4fdc22f3d2c
- Status changed from new to needs_review
comment:4 Changed 7 years ago by
- Commit changed from 1cd2a3df5b993c9593a75d94a8dcd4fdc22f3d2c to 3fe29188a7f5a747954600e551107f4d0f7b5f4f
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
3fe2918 | Wrap Python functions in PARI closures
|
comment:5 Changed 7 years ago by
- Description modified (diff)
- Summary changed from Wrap Python functions in a PARI entree and t_CLOSURE to Wrap Python functions in a PARI t_CLOSURE
comment:6 Changed 7 years ago by
- Cc pbruin added
comment:7 Changed 7 years ago by
- Commit changed from 3fe29188a7f5a747954600e551107f4d0f7b5f4f to 54a322a90f6dd55b4d38e5f2a22d4cc4f7c2d900
Branch pushed to git repo; I updated commit sha1. New commits:
54a322a | Improve documentation
|
comment:8 Changed 7 years ago by
- Commit changed from 54a322a90f6dd55b4d38e5f2a22d4cc4f7c2d900 to 908980802369b07209740329aca6a2f6710f6cec
Branch pushed to git repo; I updated commit sha1. New commits:
9089808 | Merge tag '6.7.beta4' into t/18052/wrap_python_functions_in_a_pari_entree_and_t_closure
|
comment:9 Changed 7 years ago by
I tried to implement a variadic version of this, and ran into #18623.
comment:10 Changed 7 years ago by
Two comments on the current implementation:
- I think it is better to use
ulong
instead ofunsigned long
, becauseunsigned long
may be smaller than the size of a pointer (on 64-bit Windows maybe?). - The case where the Python function returns
None
is not handled correctly; I propose--- a/src/sage/libs/pari/closure.pyx +++ b/src/sage/libs/pari/closure.pyx @@@ -65,7 +65,10 @@ cdef inline GEN call_python_func_impl "call_python_func"(GEN* args, object py_fu # Call the Python function r = PyObject_Call(py_func, t, <dict>NULL) - # Convert the result to a gen and copy it to the PARI stack + # Convert the result to a GEN and copy it to the PARI stack, + # treating None specially + if r is None: + return gnil return gcopy(objtogen(r).g) # We rename this function to be able to call it with a different
comment:11 Changed 7 years ago by
- Commit changed from 908980802369b07209740329aca6a2f6710f6cec to c14da15b4cd3c42e3d380edfc685d488ddfcd884
comment:12 Changed 7 years ago by
- Reviewers set to Peter Bruin
- Status changed from needs_review to positive_review
This looks good to me now. We can always implement a variadic version later, but I think is not a priority at the moment. Also, the most naïve version of this makes the "cube" doctest fail, because even PARI doesn't support variadic functions in apply
:
gp> g(v[..]) = v[1]^3 %1 = (v[..])->v[1]^3 gp> g(2) %2 = 8 gp> apply(g, [1,2,3]) *** at top-level: apply(g,[1,2,3]) *** ^---------------- *** apply: incorrect type in apply (t_CLOSURE).
(The error only arises because the closure is variadic; non-variadic closures do work.)
comment:13 Changed 7 years ago by
- Branch changed from u/jdemeyer/wrap_python_functions_in_a_pari_entree_and_t_closure to c14da15b4cd3c42e3d380edfc685d488ddfcd884
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Wrap Python functions in PARI closures