-
Notifications
You must be signed in to change notification settings - Fork 2.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
Make py::vectorize() ignore non-vectorizable arguments & support member functions #762
Conversation
detail::vectorize_helper<Func, Return, Args...> | ||
vectorize(const Func &f, Return (*) (Args ...)) { | ||
return detail::vectorize_helper<Func, Return, Args...>(f); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this into detail
as it didn't seem to be an intentionally exposed part of the API.
I'm wondering if this is the nicest way of solving the issue with #761. I think the cleanest solution would be for |
Yeah, I'm not entirely happy with how much needed to be added to make this work. Making it skip anything non-array-convertible would indeed be nicer. |
(Though it would still need some of this code--the current |
I'm not really seeing how this could work in practice. Say |
Something like |
6c78bd5
to
5b194b6
Compare
I've updated the PR; copying and pasting the main commit message here: This extends py::vectorize to automatically pass through non-vectorizable arguments. This removes the need for the documented "explicitly exclude an argument" workaround. Vectorization now applies to arithmetic, std::complex, and POD types, passed as plain value or by const lvalue reference (previously only pass-by-value types were supported). Non-const lvalue references and any other types are passed through as-is. Functions with rvalue reference arguments (whether vectorizable or not) are explicitly prohibited: an rvalue reference is inherently not something that can be passed multiple times and is thus unsuitable to being in a vectorized function. The vectorize returned value is also now more sensitive to inputs: previously it would return by value when all inputs are of size 1; this is now amended to having all inputs of size 1 and 0 dimensions. Thus if you pass in, for example, [[1]], you get back a 1x1, 2D array, while previously you got back just the resulting single value. Vectorization of member function specializations is now also supported via This commit also removes the documentation about manually dealing with buffers; that feature is much better covered by the direct access implementation (which is the subsequent section). |
5b194b6
to
e2acf16
Compare
cfd0d23
to
67d9614
Compare
67d9614
to
1c75420
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, just a minor comment about the docs.
docs/advanced/pycpp/numpy.rst
Outdated
} | ||
); | ||
|
||
In cases where the computation is too complicated to be reduced to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might still be good to keep the piece of code below as an complete self-contained example how how to deal with arrays when a function being vectorized is not a universal function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Looking at it now, I don't know why I removed it; I'm guessing it was a mistake of just removing too much along with the above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, scratch that. I took it out deliberately, but on second thought I see the merits in keeping it (it doesn't only apply to py::array
arguments); I've added it back in.
@@ -504,6 +512,14 @@ struct is_input_iterator<T, void_t<decltype(*std::declval<T &>()), decltype(++st | |||
/// Ignore that a variable is unused in compiler warnings | |||
inline void ignore_unused(const int *) { } | |||
|
|||
/// Apply a function over each element of a parameter pack | |||
#ifdef __cpp_fold_expressions | |||
#define PYBIND11_EXPAND_SIDE_EFFECTS(PATTERN) (((PATTERN), void()), ...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea!
// - BIndex is a incremental sequence (beginning at 0) of the same size as VIndex, so that | ||
// we can store vectorized buffer_infos in an array (argument VIndex has its buffer at | ||
// index BIndex in the array). | ||
template <size_t... Index, size_t... VIndex, size_t... BIndex> object run( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, that's a nice trick (with select_indices
).
This allows calling of functions (typically void) over a parameter pack, replacing usage such as: bool unused[] = { (voidfunc(param_pack_arg), false)..., false }; (void) unused; with a much cleaner: PYBIND11_EXPAND_SIDE_EFFECTS(voidfunc(param_pack_arg));
Adds `remove_reference_t` and converts various `typename std::remove_reference<...>::type` to using it.
Currently if you construct an `array_t<T, array::f_style>` with a shape but not strides you get a C-style array; the only way to get F-style strides was to calculate the strides manually. This commit fixes that by adding logic to use f_style strides when the flag is set. This also simplifies the existing c_style stride logic.
This extends py::vectorize to automatically pass through non-vectorizable arguments. This removes the need for the documented "explicitly exclude an argument" workaround. Vectorization now applies to arithmetic, std::complex, and POD types, passed as plain value or by const lvalue reference (previously only pass-by-value types were supported). Non-const lvalue references and any other types are passed through as-is. Functions with rvalue reference arguments (whether vectorizable or not) are explicitly prohibited: an rvalue reference is inherently not something that can be passed multiple times and is thus unsuitable to being in a vectorized function. The vectorize returned value is also now more sensitive to inputs: previously it would return by value when all inputs are of size 1; this is now amended to having all inputs of size 1 *and* 0 dimensions. Thus if you pass in, for example, [[1]], you get back a 1x1, 2D array, while previously you got back just the resulting single value. Vectorization of member function specializations is now also supported via `py::vectorize(&Class::method)`; this required passthrough support for the initial object pointer on the wrapping function pointer.
1c75420
to
92a75e2
Compare
LGTM! |
Edit: PR amended from this initial description, see below.
This commit adds py::vectorize() support for class methods, so that you can write:
Currently the only way to do this is via a lambda method wrapper with the exclusion argument technique described in the documentation. That way still works—this PR also adds a test for it—and is still needed if you want to wrap a lambda method wrapper.
Fixes #761