ENH: _lib: add common machinery for low-level callback functions#6509
Merged
Conversation
pv
force-pushed
the
ccallbacks
branch
4 times, most recently
from
9d2f016 to
a80c801
Compare
pv
changed the title
Member
Author
|
I think this is more or less ready, comments welcome. |
pv
added
enhancement
Contributor
|
It looks like a really great work. I wonder if it would be possible to make an extended tutorial for |
Member
We never put anything there, but in this case I think I agree - there's no other obvious place. |
Member
@nmayorov the last commit adds documentation which looks quite nice to me. See https://926-1460385-gh.circle-artifacts.com/0//home/ubuntu/scipy/doc/build/html-scipyorg/ccallback.html |
Member
Member
Author
|
For reviewing, I would suggest starting by looking at:
|
person142
reviewed
Sep 19, 2016
| ------------------------------------------- | ||
| user_data = ctypes.c_double(shift) | ||
| ptr = ctypes.cast(ctypes.pointer(user_data), ctypes.c_void_p) | ||
| callback = LowLevelCallable.from_cython(example, "transform", ptr) |
Member
There was a problem hiding this comment.
Doesn't this example also require adding a .pxd file to export the C api?
Member
|
Would it make sense to collect the examples into a single new tutorial page? Once this is in I imagine use of it is going to proliferate (seems like it's already in the works for ODE solvers), and it might save replication to have an authoritative guide to point to. |
Member
Author
|
No, I think "cdef api" does the same magic.
|
Member
Author
|
Maybe, but there are limited examples on which to demonstrate it currently.
|
Member
So it does. That's a good trick to know about. |
|
I tried building the branch but with no luck :( First I had to downgrade setuptools because of incorrect path quoting (IIUC) after which it got a lot further but finally failed with the below errors: NB: I quite possibly wasn't doing something right but thought I'd post the error in case it points to an actual problem with the windows build |
Member
Author
|
@dhirschfeld: may be just missing |
|
Thanks @pv - that fixed the compilation so I can now test this branch on py35/win64. I'm very interested in this branch as I have an expensive integral which I'm hoping to speedup using the numba In [4]: from scipy.integrate import quad
In [5]: def f1(x, mu):
...: return np.exp(-x**2)/(1+np.exp(-(x-mu)))
In [6]: quad(f1, -np.inf, np.inf, args=(1.1,))
Out[6]: (0.47866266846942096, 5.656862062202448e-09)
In [7]: quad(f1, -np.inf, np.inf, args=(1.9,))
Out[7]: (0.26575640518956256, 2.7307507646266408e-09)I implemented this using numbas In [58]: from numba import types
...: from numba import carray, cfunc, float64
...: from scipy import LowLevelCallable
In [59]: @cfunc(float64(float64, types.voidptr))
...: def f2(x, ptr_mu):
...: mu = carray(ptr_mu, (1,), dtype=float64)[0]
...: return np.exp(-x**2)/(1+np.exp(-(x-mu)))
In [60]: mu = ctypes.c_double(1.1)
...: mu_ptr = ctypes.cast(ctypes.pointer(mu), ctypes.c_void_p)
...: f = LowLevelCallable(f2.ctypes, mu_ptr)
In [61]: quad(f, -np.inf, np.inf)
Out[61]: (0.47866266846942096, 5.656862062202448e-09)
In [62]: mu = ctypes.c_double(1.9)
...: mu_ptr = ctypes.cast(ctypes.pointer(mu), ctypes.c_void_p)
...: f = LowLevelCallable(f2.ctypes, mu_ptr)
In [63]: quad(f, -np.inf, np.inf)
Out[63]: (0.26575640518956256, 2.7307507646266408e-09)...with a resulting 16x speedup! In [73]: %timeit quad(f, -np.inf, np.inf)
10000 loops, best of 3: 53.4 µs per loop
In [74]: %timeit quad(f1, -np.inf, np.inf, args=(1.9,))
1000 loops, best of 3: 859 µs per loop
In [75]: quad(f1, -np.inf, np.inf, args=(1.9,))[0] == quad(f, -np.inf, np.inf)[0]
Out[75]: True🎉 |
|
...since it's slightly non-trivial it might make for a good example - but that's possibly beter located in the numba docs? cc: @pitrou |
…r than index Since several signatures can be equivalent, it's more convenient to refer to them by user-specified codes, rather than by their index in the list of signatures.
Member
Author
|
@rgommers: all the life cycle issues have been addressed, as far as I see.
3.1.2017 8.02 "Ralf Gommers" <notifications@github.com> kirjoitti:
*@rgommers* commented on this pull request.
------------------------------
In scipy/_lib/src/ccallback.h <#6509>:
+ callback_obj2 = PyObject_CallMethod(lowlevelcallable_type, "_parse_callback",
+ "O", callback_obj);
+ if (callback_obj2 == NULL) {
+ goto error;
+ }
+
+ callback_obj = callback_obj2;
+
+ if (PyCapsule_CheckExact(callback_obj)) {
+ capsule = callback_obj;
+ }
+ }
+
+ if (PyCallable_Check(callback_obj)) {
+ /* Python callable */
+ callback->py_function = callback_obj;
@pv <https://github.com/pv> this comment still seems open, not clear to me
that it was addressed
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6509>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACI5mfL3o2vyhIRBq2YSHKmPNcrBsFrks5rOeRCgaJpZM4JqN_5>
.
|
Member
|
Okay, thanks for confirming. |
Member
|
Let's get this in then - has been thoroughly reviewed, seems to be in good shape, and would be good to have it in master for a few weeks before branching 0.19.x. |
Member
|
Or do you still want to do anything about this comment regarding numba ints:#4933 ? |
Member
Author
|
I added adfitional signatures for different int sizes, so it should work on
different platforms.
4.1.2017 5.24 ap. "Ralf Gommers" <notifications@github.com> kirjoitti:
… Or do you still want to do anything about this comment regarding numba
ints:#4933 <#4933> ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6509 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AACI5honufxpGMyOl38G3vzpB2aLdslWks5rOxDigaJpZM4JqN_5>
.
|
Member
|
Okay in it goes. Thanks Pauli! And thanks everyone for the reviews! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the various approaches to combined Python and low-level compiled callback functions (e.g. in integrate.quad, ndimage, ...) with something better.
We need this part of a FFI layer in Scipy, because the FFI solutions in Python are not standard, and we want to interoperate with all of them. Moreover, FFI is not standard even across Scipy, and we want a reusable solution.
Supported are:
Caller/thunk code can be implemented in C or Cython.
I ended up with a design where the callback functions need to be explicitly wrapped by the user in
LowLevelCallableobject. This is because parsing e.g. ctypes callback signatures can take a few µs, so users may want to "precompile" them. Another reason is that if sometime in the future, PyPy becomes more important and Scipy switches to cffi etc, changing the design probably is easier.The specific use cases in integrate.quad and ndimage still handle also the current "legacy" formats, to preserve backward compat.
Fixes gh-4831, gh-5002
_ccallback.cto Cython? --- no need to do that in the end, leave some test code in C as a coding example