Eigen/numpy referencing #610

jagerman · 2017-01-21T00:27:47Z

pybind's current Eigen support relies on copying whenever transferring data back and forth between Eigen C++ types and numpy arrays. In many cases we can avoid such copies by either having Eigen reference a numpy array data, or having numpy reference eigen's data. This PR does that.

(I'd like to leave it here for a while for people to comment/criticize/etc. before merging!)

Full details are in the documentation commit, but the basic highlights are:

when returning an rvalue Eigen concrete type (e.g. MatrixXd), we now return a numpy array without copying which references the MatrixXd data. The MatrixXd itself gets stashed into a capsule which is tied to the lifetime of the returned array.
when returning a reference (e.g. const MatrixXd &) we copy by default, but you can get a numpy reference instead via return_value_policy::reference or a reference tied to the parent via ::reference_internal.
when passing a numpy array to a function taking an Eigen::Ref<MatrixXd> or Eigen::Ref<const MatrixXd>, we avoid copying if possible. Unfortunately it is often not possible: Eigen::Ref by default assumes a contiguous inner stride, which means contiguous columns for a MatrixXd. Two possible solutions:
- Thus you either need to pay attention to the order of passed numpy arrays (i.e. creating with order='F'), or change Eigen types from MatrixXd to Matrix<double, Dynamic, Dynamic, RowMajor>. But even this isn't always enough: numpy's a.transpose(), for example, returns a view into the original numpy array, but just flips the column/row majors.
- A nicer solution is to take arguments as Eigen::Ref<Eigen::MatrixXd, 0, Eigen::Stride<Eigen::Dynamic, Eigen::Dynamic>>. In addition to numpy arrays of either row/column contiguity, it also allows non-contiguous inputs such as array slices. Since this is really cumbersome to write, I added py::EigenDRef<MatrixType>/py::EigenDMap<MatrixType>/py::EigenDStride as useful shortcuts.
mutable Eigen arguments (e.g. Eigen::Ref<MatrixXd>) are supported. Unlike non-mutable references (e.g. Eigen::Ref<const MatrixXd>) you won't be allowed to call such a function if a conversion of copy is required.
you can return Eigen::Blocks, Eigen::Maps, and Eigen::Refs when return a numpy array that references the block/map/matrix data without copying it. You'll very often want to use a py::return_value_policy::reference_internal here (if a method returning internal data) or some other keep-alive mechanism, for obvious reasons. You can explicitly ask for data to be copied into a new numpy array by using return_value_policy::copy.
we honour flags.writeable: you can't pass a flags.writeable = False numpy array as an Eigen::Map<Matrix> argument. We also honour the other way: if you return a const Matrix value, reference, or Eigen::Ref, you end up with an array with flags.writeable = False.
you can even do some crazy nested numpy-eigen-numpy-eigen-... referencing, if you feel so inclined (one of the added tests does this just to see that it works).

This PR also contains a few other mostly-related odds and ends that I ran into when writing all of this up.

ludwigschmidt · 2017-01-21T03:15:19Z

Looks really awesome! Two questions:

I didn't know about Eigen::Ref<Eigen::MatrixXd, 0, Eigen::Stride<Eigen::Dynamic, Eigen::Dynamic>> before. Does it have (significant) running time overhead compared to using a fixed and known row- or column-major ordering?
Mutable arguments fail if a copy would be required. Is it possible to make this behavior available for immutable arguments, too? In some cases (e.g., very large matrices), it might be better to produce an error than to copy the large matrix. In other settings a copy is certainly not an issue. So it might be best if the Eigen / numpy bindings supported both.

jagerman · 2017-01-21T03:36:18Z

@ludwigschmidt - it likely will have some impact, depending on what you're doing with the Ref, as vectorization will be disabled, which can be a big deal (depending on your CPU capabilities, of course). Another option is to keep all the actual storage in Eigen (with everything in numpy just a view on Eigen data) in which case you don't need the fully Dynamic Ref.

I'll ponder the fail-instead-of-copy issue.

yesint · 2017-01-21T09:36:10Z

Looks great!

Thus you either need to pay attention to the order of passed numpy arrays (i.e. creating with order='F'), or change Eigen types from MatrixXd to Matrix<double, Dynamic, Dynamic, RowMajor>.

This is an a very important point! End user is likely to have no idea about different storage order so the converter should provide the most logical and expected behavior. At the same time signatures of exposed C++ functions should not change just because we are making bindings for them.
So, I think the best default behavior is:

When returning Eigen matrices to create numpy array where first index is col of Eigen matrix (referencing the same data is possible).
When getting numpy arguments expect that in numpy array first index is column number.

jagerman · 2017-01-21T15:06:38Z

@yesint: when returning from Eigen to numpy there is no problem: numpy will accept any stride so we can always map into an Eigen matrix (or Ref/Map/Block).

When going the other way, swapping indices (which is essentially doing a transpose on the argument) seems rather unexpected. It also won't work in some cases: e.g. a mult(a, a.transpose()) would either get two a or two a.transpose(), neither of which can be multiplied together.

One thing I should point out, though, is that Eigen itself will make a copy if you try to pass a storage-incompatible matrix into a Ref<const M>, so we're not really doing something totally unexpected here.

patrikhuber · 2017-01-21T21:47:50Z

docs/advanced/cast/eigen.rst

+
+.. code-block:: cpp
+
+    m.def("scale", [](py::EigenRef<Eigen::MatrixXd> m, double c) { m *= c; });


I think you probably mean to use py::EigenDRef here?

Whoops, yeah. I started with EigenRef, then added the D (for Dynamic) to make it clear it isn't just the typical Eigen::Ref<M>.

patrikhuber · 2017-01-21T21:49:45Z

docs/advanced/cast/eigen.rst

+provides a ``py::EigenDRef<MatrixType>`` type alias for your convenience (along
+with EigenDMap for the equivalent Map, and EigenDStride for just the stride
+type).
+


I'm not sure I understand from this documentation why we need a fully dynamic stride type - why isn't Eigen::RowMajor sufficient to get the same layout as numpy?
Edit: Ah I think I see now - this changes the stride, but not actually the memory layout - so it's less intrusive!

Numpy doesn't always have a row-major layout, either. E.g. if a is row-major, then a.transpose() will be column major.

Also the stride and the memory layout are the same concept: Row major really just means the column stride is 1 (i.e. rows are contiguous), and column major just means the row stride is 1 (i.e. columns are contiguous). The restriction in Eigen is essentially that Ref<Matrix> requires an inner stride of 1 (where inner = column for row-major and = row for column-major).

(When comparing to numpy strides, numpy strides are in bytes, so multiply by the dtype size when looking at numpy array strides).

patrikhuber · 2017-01-21T21:51:14Z

docs/advanced/cast/eigen.rst

+or columns of a row-major matrix), but not along the inner dimension.
+
+This type, however, has the added benefit of also being able to map numpy array
+slices.  For example, you could use the following (contrived) example uses


Small typo, remove "uses" or "you could use"

patrikhuber · 2017-01-21T22:04:27Z

This looks awesome! I'll test it in my project over the next few days/weeks!

A few remarks/questions:

when returning an rvalue Eigen concrete type (e.g. MatrixXd)

I'm absolutely not sure here but wouldn't that be an lvalue?

if you return a const Matrix value, reference, or Eigen::Ref, you end up with an array with flags.writeable = False

In that case, why "when returning a reference (e.g. const MatrixXd &) we copy by default"? (Just curious or maybe I don't fully understand - probably there's a perfectly valid reason for it).

Do I understand correctly that passing a np-array to a C++ function that takes a MatrixXd& or const & would occur a copy? So in this case the general advice would be to change a C++ API to use Eigen::Ref instead of & / const &? I guess you've thought about that and there's a reason for it but is it not possible to get the same behaviour with just normal references, without having to resort to Eigen::Ref's everywhere?

This might be a stupid question... but: What happens with objects containing Eigen matrices, when these objects are passed by const&? Let's say I have

class my_class {
    MatrixXd big_matrix;
}

and I'm binding a function

f(const my_class& my) { ... };

When calling this function from Python, does big_matrix get copied? (my_class is exposed to Python via pybind11 as well)

Thank you very much for your great work on this!

jagerman · 2017-01-21T23:11:14Z

when returning an rvalue Eigen concrete type (e.g. MatrixXd)
I'm absolutely not sure here but wouldn't that be an lvalue?

No, for ordinary object returns such as MatrixXd foo() { return MatrixXd::Random(5, 5); }; we capture the MatrixXd as an rvalue and move it into a new temporary MatrixXd.

In that case, why "when returning a reference (e.g. const MatrixXd &) we copy by default"? (Just curious or maybe I don't fully understand - probably there's a perfectly valid reason for it).

Mainly because that's how lvalue reference returns work in the rest of pybind (with the default return_value_policy::automatic). The justification is that we really can't know how long the lvalue reference is going to stay valid, so you need to take an extra step in the binding code to say "no, really, it's safe to keep using this" by explicitly binding with return_value_policy::reference or "no, really, it's safe to keep using this as long as you also keep the object itself alive" by using return_value_policy::reference_internal.

Do I understand correctly that passing a np-array to a C++ function that takes a MatrixXd& or const & would occur a copy? So in this case the general advice would be to change a C++ API to use Eigen::Ref instead of & / const &? I guess you've thought about that and there's a reason for it but is it not possible to get the same behaviour with just normal references, without having to resort to Eigen::Ref's everywhere?

Yes, you're correct. The issue is that in order to provide a MatrixXd instance to a function, we have to have a MatrixXd instance in the first place, but we don't, we just have a chunk of data in memory with a map of how to interpret that data. MatrixXd, on the other hand, is a concrete class that always owns its own data: being able to reference some other data is the entire point of Ref<MatrixXd>. This is no different than Eigen itself, though: if you try to pass something that isn't exactly a MatrixXd to a function taking a const MatrixXd &, you'd invoke a copy. For example, while you couldn't pass a Matrix4d without a copy, you can pass the Matrix4d to a Ref<const MatrixXd> without needing a copy.

What happens with objects containing Eigen matrices ...

That won't be a problem at all: pybind11 internally works by storing a C++ instance of your my_class inside the wrapper that is exposed to Python; when a method is invoked, we call the C++ method on that internal instance; no copy is needed.

jagerman · 2017-01-22T05:39:08Z

Removed the gcc-7 fix commit (I included it in PR #611, which was changing related code anyway), and incorporated @patrikhuber's doc fixes.

patrikhuber · 2017-01-22T18:54:50Z

@jagerman Cool! Thank you very much for your explanations. Thanks in particular for the explanation regarding return_value_policy and the reference-passing - these are explained very clearly!

dean0x7d

Awesome! Great handling of copy/move/reference with return_value_policy and also const-correctness with the writeable flag.

Regarding Eigen::Ref arguments to constant data with fixed strides, I feel like this should never make a copy. Eigen itself wouldn't make a copy there to adjust for the strides, and I would imagine most people use Ref specifically to avoid a copy so that might be surprising behavior.

dean0x7d · 2017-01-22T20:34:12Z

include/pybind11/eigen.h

+#if EIGEN_VERSION_AT_LEAST(3,3,0)
+using EigenIndex = Eigen::Index;
+#else
+using EigenIndex = std::ptrdiff_t;


Should be using EigenIndex = EIGEN_DEFAULT_DENSE_INDEX_TYPE since it's user-configurable.

dean0x7d · 2017-01-22T20:49:21Z

include/pybind11/pybind11.h

@@ -132,9 +132,7 @@ class cpp_function : public function {
                                        ? &rec->data : rec->data[0]);

            /* Override policy for rvalues -- always move */


Comment is not accurate any more.

Fixed: s/always/usually to enforce rvp::move on an rvalue/

dean0x7d · 2017-01-22T20:59:00Z

tests/test_eigen.cpp

+    m.def("get_rm_const_ref", []() { return Eigen::Ref<const MatrixXdR>(get_rm()); });
+    // Just the corners (via a Map instead of a Ref):
+    m.def("get_cm_corners", get_cm_corners);
+    m.def("get_cm_corners_const", get_cm_corners_const);


Why not also use lambdas here (and everywhere), like above? Should reduce duplication and make things easier to follow.

Agreed, updated.

I was thinking everywhere: choleskyn, incr_matrix, even_rows etc. As far as I can see only add_rm() and add_cm() are ever used more than once.

Yeah, probably worthwhile. I was just changing the ones I added, but I'll go change the rest, too.

dean0x7d · 2017-01-22T21:42:24Z

include/pybind11/eigen.h

-template <typename T> using is_eigen_ref = is_template_base_of<Eigen::RefBase, T>;
+//template <typename T> struct is_eigen_ref : std::false_type {};
+//template <typename PlainType, int Options, typename Stride> struct is_eigen_ref<Eigen::Ref<PlainType, Options, Stride>>
+//    : std::true_type {};


Remove commented out code?

Whoops, yeah, fixed.

dean0x7d · 2017-01-22T21:45:22Z

include/pybind11/eigen.h

+//    operator Type*() { return value.get(); }
+//    operator Type&() { if (!value) pybind11_fail("Eigen::Ref<...> value not loaded"); return *value; }
+//    template <typename _T> using cast_op_type = pybind11::detail::cast_op_type<_T>;
+//};


Accidental leftover?

Yeah, fixed.

dean0x7d · 2017-01-22T22:17:04Z

@jagerman said:
One thing I should point out, though, is that Eigen itself will make a copy if you try to pass a storage-incompatible matrix into a Ref<const M>, so we're not really doing something totally unexpected here.

AFAIK The only time Ref<const M> is allowed to create a temporary is when accepting an expression. Then it evaluates that expression into a compatible format. But if it's assigned a concrete matrix, it's not going to accept incompatible strides.

dean0x7d · 2017-01-22T22:23:09Z

@jagerman said:
(I'd like to leave it here for a while for people to comment/criticize/etc. before merging!)

This PR also contains a few other mostly-related odds and ends that I ran into when writing all of this up.

Consider moving the smaller fixes into another PR which can be merged quickly. It would benefit other PRs and avoid possible merge conflicts between parallel work.

jagerman · 2017-01-22T22:26:18Z

AFAIK The only time Ref is allowed to create a temporary is when accepting an expression.

The docs say it'll copy, and it indeed appears to:

#include <Eigen/Core>
#include <iostream>

using namespace Eigen;

void f(Ref<const MatrixXd> x) { std::cout << x(0, 1) << "\n"; }

int main() {
    Matrix<double, 4, 4, RowMajor> x;
    for (int i = 0; i < 16; i++) x(i / 4, i % 4) = i;
    f(x);
}

print 1.

jagerman · 2017-01-22T22:45:40Z

Re: copying on a Ref<const Matrix>, my main concern is that there are likely legitimate C++ interfaces out there that accept arguments by Ref<const Matrix> as an accepts-anything-conformable argument, because indeed Eigen will make a copy for expressions or comformable-but-not-stride-compatible types.

So essentially we're trading off pybind11 making the interface more restrictive to python than it is to C++ against the legitimate desire to not want a copy.

I originally started playing around with argument passing to think about ways to have a sort of argument_value_policy. (This let to me getting sidetracked with #611 being the result), so that you could say something like:

.def("whatever", &Class::whatever, py::arg("x") = argument_policy::allow_copy

which would essentially resolve this by letting the pybind11 layer indicate whether it wants copyability or not (either default to allowing copy with a arg_policy::no_copy, or default to disallowing with arg_policy::allow_copy).

ludwigschmidt · 2017-01-22T22:57:36Z

So essentially we're trading off pybind11 making the interface more restrictive to python than it is to C++ against the legitimate desire to not want a copy.

I agree that there are valid reasons for either approach. Leaving the decision to the pybind user via argument policies would probably be ideal.

jagerman · 2017-01-22T22:59:11Z

Rebased with @dean0x7d 's comments integrated, and with the commits that this PR doesn't depend on moved out to #613 and #614.

jagerman · 2017-01-22T23:01:40Z

@ludwigschmidt - the issue, however, is that right now there is no such policy, and adding one potentially imposes a cost on every single bound function. Return value policies are useful all over the place; at the moment, however, only eigen would benefit from a hypothetical argument policy.

ludwigschmidt · 2017-01-22T23:04:20Z

Regarding Ref and temporary copies: it looks like Ref<MatrixXf> is not allowed to copy but const Ref<const MatrixXf>& is allowed to copy. See

https://eigen.tuxfamily.org/dox-devel/classEigen_1_1Ref.html

On that page, the Eigen documentation also says that keeping the inner stride flexible can lead to significantly slower code. So in some settings it might be good to avoid EigenDRef. They suggest a workaround based on function overloading. Is such an overloading approach also possible via pybind?

ludwigschmidt · 2017-01-22T23:07:24Z

@jagerman I see, that's a good point. Adding running time overhead everywhere is indeed an issue.

When reading the pybind documentation, I saw a section on call policies:

http://pybind11.readthedocs.io/en/master/advanced/functions.html#additional-call-policies

Could that be an approach that avoids new overhead? Unfortunately I don't know much about the pybind internals.

jagerman · 2017-01-22T23:13:04Z

Regarding Ref and temporary copies: it looks like Ref<MatrixXf> is not allowed to copy but const Ref<const MatrixXf>& is allowed to copy.

The latter doesn't technically have to be const or an lvalue (it's the const on the MatrixType that is important), but yes, you're right that Ref<Matrix> doesn't allow copies: this PR enforces that, too.

dean0x7d · 2017-01-22T23:39:22Z

The docs say it'll copy, and it indeed appears to:

Interesting. On the other hand, Matrix<double, 4, 1> -> Ref<const Matrix<double, 1, 4>> is a compile-time error even though both hold simple contiguous data.

I originally started playing around with argument passing to think about ways to have a sort of argument_value_policy. (This let to me getting sidetracked with #611 being the result), so that you could say something like:

.def("whatever", &Class::whatever, py::arg("x") = argument_policy::allow_copy

Overloading arg assignment like this doesn't seem very nice, but I can't say I have a better suggestion.

jagerman · 2017-01-22T23:50:07Z

Overloading arg assignment like this doesn't seem very nice, but I can't say I have a better suggestion.

I'm not particularly tied to that implementation. py::arg_nocopy("name") could work as well.

jagerman · 2017-02-05T23:47:18Z

Just replaced the EigenNoCopyRef with a .noconvert()-respecting implementation to go with the just-merged #634.

aldanor · 2017-02-06T19:24:38Z

Hmm, with this and #634 in mind, should array ctors force-set writeable flag to false if conversion took place? Or even rather, only allow writeable on arrays passed in as non-convert args? ("lvalues")

jagerman · 2017-02-06T19:53:21Z

Conversion just shouldn't happen in the first place with noconvert(): they should fail to load entirely if loaded with convert = false by returning false from load(). If no other overload picks it up without conversion, and it doesn't have an explicit .noconvert(), it'll get called again with convert = true. At that point, I don't see any reason to expect writeable to be false: post-conversion we have a new array with new data that should be usable normally.

jagerman · 2017-02-06T19:54:19Z

(That useless-almost-everywhere bool in load() has a use now!)

wjakob · 2017-02-14T10:29:34Z

This rocks! (especially with the new bool convert). A few minor comments:

It would be good to mention in the docs that the a returned Eigen::Matrix is stashed into a capsule so that the array can reference it without lifetime issues. (that is pretty cool btw)
I'm wondering if 1. adds potentially too much overhead for trivial kinds of situations. Imagine a program that does computations with 3D vectors -- these should probably be copied rather than using such a fancy copy-avoiding approach. (Not that this kind of program would run particularly efficiently before, but adding a big extra slowdown would be unfortunate)
The docs are amazing!! 👍

EDIT: How would you like me to commit this? Squash, or rebase?

jagerman · 2017-02-14T19:13:32Z

I'm wondering if 1. adds potentially too much overhead for trivial kinds of situations.

I'm trying out some benchmarks to get an idea of this. Will report back shortly.

EDIT: How would you like me to commit this? Squash, or rebase?

I'll squash it into fewer commits (after figuring out the above) and rebase to current master, and let you know once I think it's ready to go.

Currently when we do a conversion between a numpy array and an Eigen Vector, we allow the conversion only if the Eigen type is a compile-time vector (i.e. at least one dimension is fixed at 1 at compile time), or if the type is dynamic on *both* dimensions. This means we can run into cases where MatrixXd allow things that conforming, compile-time sizes does not: for example, `Matrix<double,4,Dynamic>` is currently not allowed, even when assigning from a 4-element vector, but it *is* allowed for a `Matrix<double,Dynamic,Dynamic>`. This commit also reverts the current behaviour of using the matrix's storage order to determine the structure when the Matrix is fully dynamic (i.e. in both dimensions). Currently we assign to an eigen row if the storage order is row-major, and column otherwise: this seems wrong (the storage order has nothing to do with the shape!). While numpy doesn't distinguish between a row/column vector, Eigen does, but it makes more sense to consistently choose one than to produce something with a different shape based on the intended storage layout.

`is_template_base_of<T>` fails when `T` is `const` (because its implementation relies on being able to convert a `T*` to a `Base<U>*`, which won't work when `T` is const). (This also agrees with std::is_base_of, which ignores cv qualification.)

A few of pybind's numpy constants are using the numpy-deprecated names (without "ARRAY_" in them); updated our names to be consistent with current numpy code.

numpy arrays aren't currently properly setting base: by setting `->base` directly, the base doesn't follow what numpy expects and documents (that is, following chained array bases to the root array). This fixes the behaviour by using numpy's PyArray_SetBaseObject to set the base instead, and then updates the tests to reflect the fixed behaviour.

Numpy raises ValueError when attempting to modify an array, while py::array is raising a RuntimeError. This changes the exception to a std::domain_error, which gets mapped to the expected ValueError in python.

Eigen::Ref objects, when returned, are almost always returned as rvalues; what's important is the data they reference, not the outer shell, and so we want to be able to use `::copy`, `::reference_internal`, etc. to refer to the data the Eigen::Ref references (in the following commits), rather than the Eigen::Ref instance itself. This moves the policy override into a struct so that code that wants to avoid it (or wants to provide some other Return-type-conditional override) can create a specialization of return_value_policy_override<Return> in order to override the override. This lets an Eigen::Ref-returning function be bound with `rvp::copy`, for example, to specify that the data should be copied into a new numpy array rather than referenced, or `rvp::reference_internal` to indicate that it should be referenced, but a keep-alive used (actually, we used the array's `base` rather than a py::keep_alive in such a case, but it accomplishes the same thing).

This commit largely rewrites the Eigen dense matrix support to avoid copying in many cases: Eigen arguments can now reference numpy data, and numpy objects can now reference Eigen data (given compatible types). Eigen::Ref<...> arguments now also make use of the new `convert` argument use (added in PR pybind#634) to avoid conversion, allowing `py::arg().noconvert()` to be used when binding a function to prohibit copying when invoking the function. Respecting `convert` also means Eigen overloads that avoid copying will be preferred during overload resolution to ones that require copying. This commit also rewrites the Eigen documentation and test suite to explain and test the new capabilities.

test_eigen.py and test_numpy_*.py have the same @pytest.requires_eigen_and_numpy or @pytest.requires_numpy on every single test; this changes them to use pytest's global `pytestmark = ...` instead to disable the entire module when numpy and/or eigen aren't available.

jagerman · 2017-02-15T07:18:17Z

It would be good to mention in the docs that the a returned Eigen::Matrix is stashed into a capsule so that the array can reference it without lifetime issues. (that is pretty cool btw)

It's already there, in the "Returning values to Python" section. I don't explicitly say it's a capsule, as that's more of an implementation detail the caller doesn't need to worry about, but the lifetime of the stored matrix is explicitly mentioned.

I'm wondering if 1. adds potentially too much overhead for trivial kinds of situations.

I ran some benchmarks on this where I construct an eigen matrix of various sizes, returning it by value: the capsule approach appears to always be faster, even for tiny matrices. I tested at -O3 and -Os, for sizes 1×1, 3×1, 3×3, 10×10, 100×100, and 1000×1000. There's an expected penalty for the largest one: the procedure with numpy copying takes about 7.5 times longer. For the smaller sizes, copying takes between 27% and 48% longer, but on all those sizes, it always takes longer.

My guess is that the copying is relatively cheap, but the memory allocation adds enough of an overhead that it's cheaper to do an Eigen move + numpy reference for any size.

EDIT: How would you like me to commit this? Squash, or rebase?

I've rebased and squashed it down now. The first 6 commits are all more or less independent features/fixes required by Eigen and, I think, properly belong as separate commits; then the main one (with code+docs+test, plus the last few miscellaneous fixes squashed together), and finally 3174f30 is a small cleanup to the test scripts to avoid needing to put the same decorator in front of almost every test in the eigen and numpy tests.

See this PR: pybind/pybind11#610

wjakob · 2017-02-24T22:14:52Z

This looks good to merge to me. Are there any other things planned, or shall I push the button?

wjakob · 2017-02-24T22:15:10Z

(and thanks for benchmarking, that's a relief)

jagerman · 2017-02-24T22:18:43Z

Nope, it's good to go!

wjakob · 2017-02-24T22:19:39Z

Ok, awesome!

jagerman mentioned this pull request Jan 21, 2017

Documentation of the Eigen wrapper unclear #573

Closed

jagerman force-pushed the eigen-numpy-referencing branch from dfef38d to 960d3d0 Compare January 21, 2017 03:34

patrikhuber reviewed Jan 21, 2017

View reviewed changes

jagerman force-pushed the eigen-numpy-referencing branch 2 times, most recently from 0b5621a to a92bbff Compare January 22, 2017 05:37

dean0x7d reviewed Jan 22, 2017

View reviewed changes

jagerman force-pushed the eigen-numpy-referencing branch from 995ac3d to a1067bd Compare January 22, 2017 22:54

jagerman mentioned this pull request Jan 22, 2017

Minor fixes #613

Merged

jagerman mentioned this pull request Feb 6, 2017

How to create a numpy array of objects? #647

Closed

jagerman added 6 commits February 15, 2017 01:51

Change numpy constants to non-deprecated versions

5539b12

A few of pybind's numpy constants are using the numpy-deprecated names (without "ARRAY_" in them); updated our names to be consistent with current numpy code.

Change array's writeable exception to a ValueError

4e517f9

Numpy raises ValueError when attempting to modify an array, while py::array is raising a RuntimeError. This changes the exception to a std::domain_error, which gets mapped to the expected ValueError in python.

jagerman force-pushed the eigen-numpy-referencing branch from 8fe1859 to 3174f30 Compare February 15, 2017 07:02

jagerman added 2 commits February 15, 2017 02:15

jagerman force-pushed the eigen-numpy-referencing branch from 3174f30 to 93c3212 Compare February 15, 2017 07:15

mwoehlke-kitware mentioned this pull request Feb 15, 2017

find NumPy module inside pybind11 RobotLocomotion/drake#5158

Merged

jagerman mentioned this pull request Feb 17, 2017

Meta: Change governance model of pybind11 project? #680

Closed

qianyizh added a commit to isl-org/Open3D that referenced this pull request Feb 20, 2017

A major upgrade on Eigen handling

3540341

See this PR: pybind/pybind11#610

wjakob merged commit 2a75784 into pybind:master Feb 24, 2017

jagerman modified the milestone: v2.1 Mar 19, 2017

qianyizh mentioned this pull request May 10, 2017

Upgrade pybind11 to 2.2+ isl-org/Open3D#18

Closed

rwgk mentioned this pull request Feb 9, 2023

FWD pybind11 google/pybind11clif#610

Closed


		.. code-block:: cpp

		m.def("scale", [](py::EigenRef<Eigen::MatrixXd> m, double c) { m *= c; });

		@@ -132,9 +132,7 @@ class cpp_function : public function {
		? &rec->data : rec->data[0]);

		/* Override policy for rvalues -- always move */

Eigen/numpy referencing #610

Eigen/numpy referencing #610

Conversation

jagerman commented Jan 21, 2017

ludwigschmidt commented Jan 21, 2017

jagerman commented Jan 21, 2017 • edited Loading

yesint commented Jan 21, 2017 • edited Loading

jagerman commented Jan 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrikhuber Jan 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrikhuber commented Jan 21, 2017 • edited Loading

jagerman commented Jan 21, 2017

jagerman commented Jan 22, 2017

patrikhuber commented Jan 22, 2017

dean0x7d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dean0x7d commented Jan 22, 2017

dean0x7d commented Jan 22, 2017

jagerman commented Jan 22, 2017

jagerman commented Jan 22, 2017

ludwigschmidt commented Jan 22, 2017

jagerman commented Jan 22, 2017

jagerman commented Jan 22, 2017

ludwigschmidt commented Jan 22, 2017

ludwigschmidt commented Jan 22, 2017

jagerman commented Jan 22, 2017

dean0x7d commented Jan 22, 2017

jagerman commented Jan 22, 2017

jagerman commented Feb 5, 2017

aldanor commented Feb 6, 2017 • edited Loading

jagerman commented Feb 6, 2017

jagerman commented Feb 6, 2017

wjakob commented Feb 14, 2017 • edited Loading

jagerman commented Feb 14, 2017

jagerman commented Feb 15, 2017 • edited Loading

wjakob commented Feb 24, 2017

wjakob commented Feb 24, 2017

jagerman commented Feb 24, 2017

wjakob commented Feb 24, 2017

jagerman commented Jan 21, 2017 •

edited

Loading

yesint commented Jan 21, 2017 •

edited

Loading

patrikhuber Jan 21, 2017 •

edited

Loading

patrikhuber commented Jan 21, 2017 •

edited

Loading

aldanor commented Feb 6, 2017 •

edited

Loading

wjakob commented Feb 14, 2017 •

edited

Loading

jagerman commented Feb 15, 2017 •

edited

Loading