Opened 6 years ago

Closed 6 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:

Status badges

Description (last modified by jdemeyer)

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 6 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • 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 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/wrap_python_functions_in_a_pari_entree_and_t_closure

comment:3 Changed 6 years ago by jdemeyer

  • Commit set to 1cd2a3df5b993c9593a75d94a8dcd4fdc22f3d2c
  • Status changed from new to needs_review

New commits:

1cd2a3dWrap Python functions in PARI closures

comment:4 Changed 6 years ago by git

  • Commit changed from 1cd2a3df5b993c9593a75d94a8dcd4fdc22f3d2c to 3fe29188a7f5a747954600e551107f4d0f7b5f4f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3fe2918Wrap Python functions in PARI closures

comment:5 Changed 6 years ago by jdemeyer

  • 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 6 years ago by pbruin

  • Cc pbruin added

comment:7 Changed 6 years ago by git

  • Commit changed from 3fe29188a7f5a747954600e551107f4d0f7b5f4f to 54a322a90f6dd55b4d38e5f2a22d4cc4f7c2d900

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

54a322aImprove documentation

comment:8 Changed 6 years ago by git

  • Commit changed from 54a322a90f6dd55b4d38e5f2a22d4cc4f7c2d900 to 908980802369b07209740329aca6a2f6710f6cec

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

9089808Merge tag '6.7.beta4' into t/18052/wrap_python_functions_in_a_pari_entree_and_t_closure

comment:9 Changed 6 years ago by pbruin

I tried to implement a variadic version of this, and ran into #18623.

comment:10 Changed 6 years ago by pbruin

Two comments on the current implementation:

  • I think it is better to use ulong instead of unsigned long, because unsigned 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 6 years ago by git

  • Commit changed from 908980802369b07209740329aca6a2f6710f6cec to c14da15b4cd3c42e3d380edfc685d488ddfcd884

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

8bd8218Merge remote-tracking branch 'origin/develop' into t/18052/wrap_python_functions_in_a_pari_entree_and_t_closure
c14da15Allow functions returning None

comment:12 Changed 6 years ago by pbruin

  • 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 6 years ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.