-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NBEP for @cfunc #1841
NBEP for @cfunc #1841
Conversation
Transcribing my review comments from chat here:
|
Current coverage is 73.19%
@@ master #1841 diff @@
==========================================
Files 288 461 +173
Lines 51589 62692 +11103
Methods 0 0
Messages 0 0
Branches 5340 6244 +904
==========================================
+ Hits 45144 45887 +743
- Misses 5535 15888 +10353
- Partials 910 917 +7
|
DyND is very interested in this as well, so putting a comment here so I get pinged. |
How would foreign C code interpret the NRT-allocated JIT class?
Yes (though I'm not sure the same would be possible with cffi).
Actually, I had not envisioned that possibility. Implementing it would need doing something like CFFI, i.e. compiling the "C function object" as a member of a C extension (rather than a plain Python class inside Numba). |
The void * pointer case would be for APIs where foreign C code does not every try to dereference the pointer, but simply passes it back to the callback as way for the callback to retain state between calls. I don't think it is particularly important at the moment, but I wanted to raise the issue. As for the ahead-of-time compilation case: Couldn't we handle |
Well,
Oh, I see, I hadn't thought about that. That's an interesting case. There are two sides to it:
|
I think we can hold the AOT compilation question for after the first implementation. One additional thing we do want to support (to minimize surprise) is calling |
We had not planned to allow I expect that in situations where you want to call the function from both an external C API and from Python, it would make sense to follow a pattern where the @jit
def mean_square_error(params, x, obs_y):
c0, c1 = params
sq_error = 0.0
for i in range(x.size):
sq_error += ((c0 + c1 * x[i]) - obs_y[i])**2
return sq_error / x.size
@cfunc(float64(intp, CPointer(float64), intp, CPointer(float64), CPointer(float64)))
def mean_square_error_callback(nparams, params, npoints, x, obs_y):
params_array = carray(params, (nparams,))
x_array = carray(x, (npoints,))
obs_y_array = carray(obs_y, (npoints,))
return mean_square_error(params_array, x_array, obs_y_array) It is quite possible that LLVM will even fuse these functions together during optimization. |
Good point for array arguments, but for simple "elementwise" functions, like "integrand" in the proposal text, this would avoid having to define silly boilerplate code, though I admit this is not a frequent use case for me. This is almost unrelated to this proposal (*) but CPointer(type) looks ugly to me/inconsistent with "voidptr". I would prefer "cptr", "ptr" or even "cpointer". (*) I mention it here because it is the first time I see it. |
Conflicts: docs/source/proposals/index.rst
OK, going to merge this now that the initial implementation is basically done. |
At what point should I attempt to do something with this and DyND? Is that point now? I'd be happy to try and update our Numba bindings, and give feedback if that's the case. |
I'd wait for the Numba 0.26 release in a few weeks. The PR implementing |
If you want to be super bleeding edge, you could build from #1871 |
No description provided.