Pybind11: Problems with integer conversions (again)

[mrzv]

I have a new set of problems with integer conversions. It's not clear to me what's going on (and it requires another library to reproduce), but hopefully it will make sense to you.

You need mpi4py for this example.

Define a function through pybind11:

m.def("print_comm", [](int x) { std::cout << "Got comm: " << x << std::endl; });

Now test it from Python:

from mpi4py import MPI
w = MPI.COMM_WORLD
a = MPI._addressof(w)
print type(a)
# prints: <type 'int'>

print_comm(a)

The last command produces:

TypeError                                 Traceback (most recent call last)
<ipython-input-7-16f94faa6020> in <module>()
----> 1 h.print_comm(a)

TypeError: Incompatible function arguments. The following argument types are supported:
    1. (int) -> None

I'm not sure how to debug this. If you have advice or any idea what's going on, I'd appreciate it.

[wjakob]

Hi Dmitry,

can you submit a reproducible example that does not depend on mpi4py? (Does it still happen if you bind a dummy function that just has the same function signature but no external dependencies?)

Also, it would be good to know what Python version this is.

Thanks,
Wenzel

[mrzv]

Hi Wenzel,

Everything I describe was under Python 2.7.11 on OS X (El Capitan). I'll try to create a stand-alone example. I don't quite understand what you mean by "binding a dummy function that just has the same function signature but no external dependencies"?

Dmitriy

[wjakob]

Never mind that last bit, I realize now that it did not make sense.

[mrzv]

Hm, so it seems Python reports the type of a incorrectly. It's actually a void*, which ought to be a long I think. If I change print_comm to accept either void* or long everything works — well, this part of the code anyway.

In short, it's not a pybind11 issue, so this can be closed. Sorry for the hassle.

[wjakob]

no problem.

[mrzv]

Actually, there is something else I don't understand. Maybe you have an idea what's going on.

What _addressof returns is the result of PyLong_FromVoidPtr(ptr) from some void* ptr.

If I change my function to accept void*, it compiles and runs, but the pointer that I get is clearly garbage. If I change it to accept long, and then do a reinterpret_cast<void*>, then I get the right pointer.

Any idea why this would be happening? Is there a conversion missing somewhere?

[wjakob]

Not 100% sure -- I can take a look if you could submit a self-contained example that demonstrates the issue.

[mrzv]

Ok, I'll try to cook one up.

[mrzv]

Actually, I didn't have to dig deep. The following already creates a problem (Python 2.7.11, OS X El Capitan):

    m.def("get_val",     [&m]()        {
                                          void* x = &m;
                                          std::cout << x << std::endl;
                                          py::handle h = PyLong_FromVoidPtr(x);
                                          return h;
                                       });
    m.def("print_val",   [](void* x)  { std::cout << x << std::endl; });
    m.def("print_val2",  [](long x)   { std::cout << reinterpret_cast<void*>(x) << std::endl; });

x = m.get_val()
m.print_val(x)
m.print_val2(x)

I get:

0x7fff52c5d0c0
0x7fff52c5ca98

It's also worth noting that it's impossible to return a void* directly from a function, but that's probably just a matter of adding a conversion.

[wjakob]

Minor thing: better to be explicit about borrow status, or you may get reference count leaks.

py::object h = py::object(PyLong_FromVoidPtr(x), false);
return h;

[mrzv]

Ah, good to know. Thanks.

[wjakob]

I've committed a fix which disallows declarations of the form

m.def("print_val",   [](void* x)  { std::cout << x << std::endl; });

(i.e. void* as a function argument)

Not knowing the type of an argument is a bottomless pit of trouble with regards to casting and reference counting, so the way to go will be to pass longs.

[mrzv]

I obviously don't understand all the nuances, but does it not make sense to add casts for void* to long when returning (using PyLong_FromVoidPtr) and from long to void* when accepting (using PyLong_AsVoidPtr). In some sense, the correct version of print_val2 should be:

    m.def("print_val2",  [](py::object x)   { std::cout << PyLong_AsVoidPtr(x.ptr()) << std::endl; });

Maybe this conversion could be done implicitly if the argument is void*? It feels dirty from the Python point of view, but it's probably the cleanest way to exchange data between C codes exposed to Python in different ways.

[ax3l]

@mrzv do you have a final snippet how you pass an mpi4py communicator (for both MPICH (int) and OpenMPI (void*)) to pybind11? I am a bit puzzled how to implement this gently in the end.

Update: went for https://github.com/openPMD/openPMD-api/pull/454

struct openPMD_PyMPICommObject
{
    PyObject_HEAD
    MPI_Comm ob_mpi;
    unsigned int flags;
};
using openPMD_PyMPIIntracommObject = openPMD_PyMPICommObject;

// in .def(py::init([](py::object &comm){
// several checks on comm...
MPI_Comm* mpiCommPtr = &((openPMD_PyMPIIntracommObject*)(comm.ptr()))->ob_mpi;

(note that MPI_Comm is unfortunately implementation dependent, so compile in one consistent stack)

[mrzv]

I ended up with something like:

from mpi4py import MPI
a = MPI._addressof(comm)

on the Python side. And then on the C++ side, I have:

      .def(py::init([](long comm_)
                       {
                           MPI_Comm comm = *static_cast<MPI_Comm*>(reinterpret_cast<void*>(comm_));
                           return new ...;
                       }), "comm"_a
          )

where I pass a from Python as comm_ here.

I haven't touched this code in years, so I don't recall all the details, but I hope it helps.

[ax3l]

Thanks for the details!
So you passed the long-casted pointer of the MPI communicator while I now forward the mpi4py wrapper class.

Sorry for digging up this old thread, it seams to be one of the few popular ones for mpi4py bindings to C/C++ :)

[mrzv]

To be honest, I don't quite understand how your conversion works. Is openPMD_PyMPICommObject supposed to look exactly like mpi4py's MPIComm object? If yes, why not take ob_mpi directly? Why are you working with the address? I'm a bit nervous about dealing in MPI_Comm*, since in MPICH, things like MPI_COMM_WORLD are just constants, but on the other hand, my code does it, too -- so it must be necessary.

It's been too long since I dug into this, I no longer remember what's going on. (Ironically, I need to resurrect this code soon, so it's a timely discussion.)

[ax3l]

Is openPMD_PyMPICommObject supposed to look exactly like mpi4py's MPIComm object?

yes, exactly. I just try to avoid including mpi4py/mpi4py.h which would provide a converter for it: MPI_Comm* mpiCommPtr = PyMPIComm_Get(comm.ptr()); Since the whole MPI ABI is not defined anyway, there is little one can due but runtime checks before this very line to ensure things do not go south doe to mismatched MPI flavors.

I'm a bit nervous about dealing in MPI_Comm*

MPI_Comm is a datatype that is in MPICH implemented as int and in OpenMPI as void*. A pointer of both is always fine if one can guarantee proper lifetime.

What I try to avoid are two things: _addressof on the user-side and compile-time mpi4py headers for this one simple type that is just a plain wrapper and unlikely to change (numpy is abstracted similarly in pybind11).

[mrzv]

Got it. Why avoid _addressof on the user side?

[ax3l]

Just subjective feeling, because it's feels a bit unnatural if the user works with comm = MPI.COMM_WORLD objects and the like :)
Python users don't necessarily know the concept of pointers and it's more verbose than necessary.

[mrzv]

Oh, but you can hide it from the user. Call _addressof from your own code, either monkey-patching the appropriate call from Python, or directly in C++. (I do the former in my code.)

[ax3l]

Uh that's true, do you usually add another python layer for monkey-patching with pybind11 projects or do you do this inside the pybind11 C++ code? I just though passing the communicator is the most simple way to avoid introducing further indirections.

[mrzv]

I almost always have a python layer anyway, even if I don't need to monkey-patch anything. It's usually helpful to add some pure python functionality. But for this use case, it's of course straightforward to just import mpi4py from C++ and do the necessary manipulation.