Skip to content
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

shared_ptr<Eigen::ArrayXd> not mapped to python #1731

Closed
XWarin opened this issue Mar 18, 2019 · 4 comments
Closed

shared_ptr<Eigen::ArrayXd> not mapped to python #1731

XWarin opened this issue Mar 18, 2019 · 4 comments

Comments

@XWarin
Copy link

XWarin commented Mar 18, 2019

Hi

The library permits to map Eigen:ArrayXd (or matrix) but not some smart_pointer of it.
For example, some mapping possible with boost::python doesn't seem to be possible.
For example the following code generates an error.

#include <Eigen/Dense>
#include
#include
#include
#include <pybind11/pybind11.h>
#include <pybind11/eigen.h>
#include <pybind11/stl_bind.h>
#include <pybind11/stl.h>

class toto
{
public:
toto(){}
void useSharedPtr( const std::shared_ptr< Eigen::ArrayXd > & i ) const
{ }
};

PYBIND11_MODULE(PyBindTest, m) {
pybind11::class_< toto >(m,"toto")
.def(pybind11::init<>())
.def("useSharedPtr", &toto::useSharedPtr)
;
}
Is there a workaround ?

@eacousineau
Copy link
Contributor

eacousineau commented Mar 20, 2019

Technical 'splanation

I believe to use a smart pointer in pybind11, it has to be a registered type (b/c it has to keep that memory somewhere).

In your case, though, you have a pure type_caster<> type (see pybind11/eigen.h + pybind11/numpy.h);
if there is no explicit type_caster<> for shared_ptr<MatrixBase<Derived>> (which I assume there isn't), then you won't be able to cast it. (It'll either puke compiler errors, or die at runtime saying that the RTTI name for shared_ptr<ArrayXd> is not registered.)

Design Question

Really, though, my main question to you is: Do you really need a shared_ptr with an Eigen type?

pybind11s functionality already permits ownership of the Eigen data when passing; e.g.:
https://github.com/pybind/pybind11/blob/25abf7e/include/pybind11/eigen.h#L244

If you want to just share the number-y bits, you can return Eigen::Ref<>.
If you really need it to be a shared_ptr<> for some lifetime semantics (but if so, why???), then maybe you can wrap it in a container class, and just bind that member as a property; for example:

struct Rawr {
  ArrayXd get_value() const { return *value; }  // May want a non-null check!
  void write_value(ArrayXd in) { *value = in; }  // Also here!
  std::shared_ptr<ArrayXd> value;
};
py::class_<Rawr>(m, "Rawr")
   .def_property("value", &Rawr::get_value, &Rawr::set_value);

Meta: If you have a chance, can you wrap your code with backticks ```? See GitHub page, specifically on the "Code" tab.

@XWarin
Copy link
Author

XWarin commented Mar 20, 2019

Thank you for your answer. Sorry for the backticks. I am not familiar with github.
As for your question why shared_ptr< Eigen::ArrayXd>. It is due to the fact that the main code is in c++
https://gitlab.com/stochastic-control/StOpt
I provide some python mapping for this library and it is convenient to manipulate sometimes some std::vector < Eigen::ArrayXd> (for example).
Of course i could have designed the whole software trying to avoid this shared_ptr, but i don't want to rewrite the whole code.
Besides, the mapping with boost python was very simple because i could register easily the shared_ptr< Eigen::ArrayXd>.
Write now with pybind, i have to use wrappers to extract and copy information and it is a regression compared to boost python (the number of lines are exploding...)
Otherwise the library is very convenient and the documentaon very well made...

@eacousineau
Copy link
Contributor

Ahh, makes sense! I've had to, uh, do some pretty odd things to enable the binding of Drake without changing the API, so I completely understand!

Reading briefly through the StOpt Boost.Python bindings, my assumption is that you want the functional equivalent of this code?
https://gitlab.com/stochastic-control/StOpt/blob/e2f7d2941197a15feb6a1db7954b31cb2429f2c2/StOpt/python/NumpyConverter.hpp

If so, I think that's easily achievable; in this case, the above code is copying the matrix data rather than aliasing it (Line 263); you can write a type_caster which binds to shared_ptr<MatrixBase<Derived>>, then just ultimately uses py::cast(*matrix_ptr) to convert from any pybind11-compatible matrix type to Python.

You can possibly alias the existing memory, but if you're not using that (which seems like not), then I'd stick with just copying the memory.

Here're the pybind11 docs: https://pybind11.readthedocs.io/en/stable/advanced/cast/custom.html

TBH, though, I generally don't use the convenience macro all that much, as I like to have more fine-tuned control (read: foot-guns).

This may be overly generic / hacky, but here's what I've done in our code base to handle Eigen::Translation<>:
https://github.com/RobotLocomotion/drake/blob/c67603cc6161db74ab58346f3a807af37c4ee5cc/bindings/pydrake/common/eigen_geometry_pybind.h#L30

You can use this with the Wrapper type; that being said, maybe it'd be easier to ditch that pattern and write the caster directly. The pybind11 docs expose the caster bits kind-of, but only for specific types (rather that templating / SFINAE :(

Let me know if you run into any hiccups! (though I may be a bit slow to respond...)

@XWarin
Copy link
Author

XWarin commented Mar 21, 2019

Thank you for you answer.
You are right : i used the NumpyConvert copying the vector and some users have cases where it is too costly. That the reason i want to switch to Pybind using some references where it is crucial.
At last i did a wrapper during my binding. It generates a lot of code but it was possible.
Thank you for the example given an the reference for adding a specific bindind with hands. I had to modify slightly some api in the C++ (hope won't make too many changes for my c++ users).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants