Pybind11: How to pass a shared pointer by reference to c++

[RiccardoRossi]

Dear list,
i am cross posting here a problem that i consulted on gitter, so to be able to post some code.

@yeganer is proposing me to creating an auxiliary class containing the pointer, but i would love to avoid it as possible

My situation is the following: i have a linear algebra library (Trilinos Epetra) that provides some distributed vector implementation. I need to implement a resize method, that is not implemented by that library. To do this, i am passing around pointers to my matrices, and when i want to resize, i allocate a new vector and i swap the pointer.
To do this i need to change the shared_ptr itself and not just its content.

so i have two types (exported as distinct types in pybind11)

  Vector
  std::shared_ptr<Vector>

and i define the functions (pseudo code, but it should be pretty close)

  std::shared_ptr<Vector> CreateEmptyVectorPointer() 
  { return std::make_shared<Vector>(0); }  

  Resize(std::shared_ptr<Vector>& self, int n)
  {
      auto pnew = std::make_shared<Vector>(n);
      self.swap(pnew);
  }

 Matrix& GetReference(std::shared_ptr<Vector>& self)
 {
    return *self;
 }

then i would like to call it from python as something like

def test_resize(self):
    comm = KratosMultiphysics.TrilinosApplication.CreateCommunicator()
    space = KratosMultiphysics.TrilinosApplication.TrilinosSparseSpace()

    print(dir(space))
    print(space)
    pb = space.CreateEmptyVectorPointer(comm)
    print("000")
    space.ResizeVector(pb,2)
    print("111")
    rb = pb.GetReference()
    print("222")
    n = space.Size(rb)
    self.assertEqual(n,0)

the actual code in which i try to do the export is here: (ported from a working boost::python code)

https://github.com/KratosMultiphysics/Kratos/pull/1513/files#diff-0924c5681634f0687c1b74c47ff15f71R279

thanks in advance for any help on this

[RiccardoRossi]

ref_to_pointer.zip

here the simplest test i could develop.

[jagerman]

I think you'll need to (and should!) use an auxiliary wrapper for this, as @yeganer suggested. This is partly about maintaining sanity on the C++ side: your "shared" pointer isn't really shared when Resize is detaching the shared nature of its ownership: any existing shared pointers aren't sharing ownership with the new, resized one anymore--which is violating what shared_ptr is all about.

As for why it won't work in pybind, there are two reasons I can think of (and quite possibly more): first is that the internal instance structure stores both a pointer and holder around that pointer. The second is that the pointer itself is a key for the set of registered instances--so that if you return a pointer to an existing, python-wrapped Foo instance we return the same Python object rather than making a new one.

Now you could try poking around in the internals if you really, really want to make this work, but I don't recommend it. (It goes without saying that this is unsupported, untested, may break in future versions, and may not actually work at all).

What you'd have to do is something like this:

   m.def("Resize", [](Vector *self, int n) {
        using MyType = Vector;
        using HolderType = std::shared_ptr<MyType>;

        // py::detail::instance is where all the pybind internal object glue lives:
        auto *inst = reinterpret_cast<py::detail::instance *>(py::cast(self).ptr());
        auto *tinfo = py::detail::get_type_info(typeid(MyType));
        // detail::value_and_holder handles accessing the non-trivial instance data layout:
        auto v_h = inst->get_value_and_holder(tinfo);
        // Delete the instance from the instance cache:
        py::detail::deregister_instance(inst, v_h.value_ptr(), tinfo);

        // Construct the new type directly in the instance:
        v_h.value_ptr<MyType>() = new MyType(n);

        if (v_h.holder_constructed()) {
            // We have an existing holder: reset it with the new pointer:
            v_h.holder<HolderType>().reset(pnew);
        }
        else {
            // We don't have a holder (i.e. this was an unowned pointer), so construct one
            // and take over ownership.  (The old pointer is left dangling, but it was *already*
            // dangling, i.e. it came from a py::return_value_policy::reference return.)
            new (&v_h.holder<HolderType>()) HolderType(pnew);
            v_h.set_holder_constructed();
        }
        // Insert the new pointer value back into the known instances cache:
        py::detail::register_instance(inst, v_h.value_ptr(), tinfo);
    }

The scary warning, and the number of times I wrote py::detail there should scare you away, though.

(Cc @EricCousineau-TRI -- I think some of the above might be helpful or at least insightful for the ownership transfer PRs).

[yeganer]

@jagerman Could I use something similar to workaround #1278 ?

[RiccardoRossi]

Dear @jagerman , first of all thank you very much for your detailed answer. I definitely agree with you that the solution you propose is dark magic.

I agree with you that using a shared_ptr for the kind of use i am doing is not nice. The problem is that it does not work even if i use a unique_ptr which would be the clean way to go.
In particular i cannot make to compile this code (which this time should be a perfectly acceptable way to go, at least from the c++ side):

#include <iostream>

#include <memory>
#include <pybind11/pybind11.h>


using namespace pybind11;

class Small
{
public: 
    Small(int value)
    {mvalue=value;}

    int GetValue(){return mvalue;}

private:
    int mvalue;
};

void Swapper(std::unique_ptr<Small>& s, int new_value)
{
    auto p = std::make_unique<Small>(new_value);
    s.swap(p);
}
PYBIND11_MODULE(aaa,m)
{
        class_<Small, std::unique_ptr<Small> >(m,"Small")
        .def(init<int>())
        .def("GetValue",&Small::GetValue)
        ;
        m.def("Change",Swapper);
}

[jagerman]

That's a limitation of how argument conversion works: we never directly expose raw access to the internal holder object; in the case of a shared_ptr we can provide you with a copy, but you aren't getting a reference to the real thing. That's why in my workaround I accepted the argument as a raw pointer, so that I could go via the internals to actually get at the internally stored pointer.

Using a wrapper here seems like it would be much easier: pybind gets a reference that doesn't change, but you can change the reference in. You don't even have to define the wrapper yourself--you can bind std::unique_ptr<Small> instead of Small (so that, internally, pybind stores a std::unique_ptr<std::unique_ptr<Small>>). Here's an implementation that ought to work:

#include <iostream>

#include <memory>
#include <pybind11/pybind11.h>


namespace py = pybind11;

class Small {
public: 
    Small(int value) : mvalue{value} {}
    int GetValue() const { return mvalue; }
private:
    int mvalue;
};

void Swapper(std::unique_ptr<Small> &s, int new_value) {
    auto p = std::make_unique<Small>(new_value);
    s.swap(p);
}

// Important: we don't want pybind to try to deference a `std::unique_ptr<Small>`:
PYBIND11_MAKE_OPAQUE(std::unique_ptr<Small>);

PYBIND11_MODULE(aaa, m) {

    py::class_<std::unique_ptr<Small>>(m, "Small")
        .def(py::init([](int n) { return std::make_unique<Small>(n); }))
        .def("GetValue", [](std::unique_ptr<Small> &self) { return self->GetValue(); })
        ;

    m.def("Change", Swapper);
}

If you have a lot of methods, writing lambdas for each one could be annoying, but you could use:

template <typename Wrapped, typename Return, typename... Args>
auto wrap_helper(Return (Wrapped::*f)(Args...)) {
    return [f](std::unique_ptr<Wrapped> &self, Args... args) { return ((*self).*f)(args...); };
}
template <typename Wrapped, typename Return, typename... Args>
auto wrap_helper(Return (Wrapped::*f)(Args...) const) {
    return [f](const std::unique_ptr<Wrapped> &self, Args... args) { return ((*self).*f)(args...); };
}

PYBIND11_MODULE(aaa, m) {
    // ...
        .def("GetValue", wrap_helper(&Small::GetValue))
    // ...
}

[RiccardoRossi]

@jagerman thank you very much for being didactic on this. (as well as for the time spent in your suggestions).

I will go for the wrapper. Thanks again!

[RiccardoRossi]

sorry, i just realized that by closing i also silences @yeganer 's question. Reopening for this, but please close as you consider

BTW, the wrapping approach did work

[jagerman]

I've already responded directly in #1278. Closing this.

:+1: on getting the wrapper approach to work for you.