4

I am unable to return the input of a class method (input: specific instances of a seperate class) to Python. The binding compiles and I can use the resulting module in Python. The class method should however return the same instances as it admits (after some processing).

The Obstacle class is used as the input. The ObstacleProcess class has a method (Python: __call__ / C++: operator_py) which processes the input (instances of Obstacle). The following Python code shows that different instances of Obstacle is returned:

import example

obstacle_1 = example.Obstacle()
obstacle_2 = example.Obstacle()

obstacles = [obstacle_1, obstacle_2]
print(obstacles)

params = example.Params()
obstacle_process = example.ObstacleProcess(params)
obstacles = obstacle_process(obstacles)
print(obstacles)

The first print returns: [<example.Obstacle object at 0x7fb65271e1b0>, <example.Obstacle at 0x7fb652735070>], whilst the second print retuns: [<example.Obstacle at 0x7fb652734670>, <example.Obstacle object at 0x7fb652735230>].

This is not the desired output as obstacle_1 initially lives at 0x7fb65271e1b0, and after the operator()/__call__ call it lives at 0x7fb652734670. I want obstacle_1 to keep its initial address of 0x7fb65271e1b0, even after the other class (ObstacleProcess) has processed obstacle_1.

The following code shows the source code is bound with pybind11:

// pybind11 binding
py::class_<Obstacle, std::shared_ptr<Obstacle>>(m, "Obstacle")
    .def(py::init<>());

py::class_<ObstacleProcess>(m, "ObstacleProcess")
    .def(py::init<
        const Params&>()
    )
    .def("__call__", &ObstacleProcess::operator_py<Params>, py::return_value_policy::reference);

The next block shows how operator_py is implemented in the source code:

template <Params>
std::vector<Obstacle>& operator_py(
    std::vector<Obstacle>& obstacles,
    const Params &parameters
)
{

    ...

    return obstacles
}

I have tried with and without std::shared_ptr<Obstacle>. The current implementation gives the same result as not using shared_ptr at all, thus there is something wrong with how I have implemented shared_ptr. I have tried to use PYBIND11_MAKE_OPAQUE(std::shared_ptr<std::vector<Obstacle>>);, but my implementation of this did not change the result.

I have not tried to use the pybind11 — smart_holder branch, maybe this branch has to be used for this case?

JMy
  • 73
  • 8
  • You're returning `std::vector` by value in `operator_py`... – Dan Mašek Jul 25 '23 at 15:45
  • 1
    Thank you for pointing that out @DanMašek! I however, unfortunately only made this mistake when typing down the MWE in my question. Do you see anything else that is wrong? – JMy Jul 25 '23 at 16:46
  • This is just a wild guess, but would a `std::vector>` make any difference? – Dan Mašek Jul 25 '23 at 16:49
  • You are thinking in operator_py() or in py::class_>(m, "Obstacle")? – JMy Jul 25 '23 at 16:50
  • `operator_py`. Because right now, the vector holds the Obstacles by value as well, which I'd expect to involve a copy when populating it. – Dan Mašek Jul 25 '23 at 16:52
  • 1
    I am actually using `std::vector>` (and not `std::vector`) , so I had to do: `std::vector, Eigen::aligned_allocator>> &obstacles`. This type is a bit confusing, so I don't know how to get "back" the `Obstacle` from this type such that I can process it. – JMy Jul 25 '23 at 17:10
  • IMHO the allocator shouldn't make any difference in how you access the elements. So `obstacles[i]`/`at`/iteration would give you individual shared pointers, which you can dereference (`*obstacles[i]`) to get the `Obstacle` instance they hold. However, given that now you have a vector of smart pointers, as opposed to vector of `Obstacle`s, I'd say there's no benefit of using `aligned_allocator` at all (since the smart pointers don't really need a non-standard alignment, and the extra level of indirection that introduce means you no longer have a flat array of `Obstacle`s in memory).. – Dan Mašek Jul 25 '23 at 18:40
  • Assuming that you had that allocator there for a good reason (i guess performance), a wholly different approach might be appropriate, although to judge that properly, I think I'd have to see a lot more context than what is currently available in your question. – Dan Mašek Jul 25 '23 at 18:44
  • 1
    I have gotten the code to compile and run in Python, but I am getting a segmentation fault when using `std::vector, Eigen::aligned_allocator>> &obstacles`. I am not quite sure what to use as input from the Python side with this new input type... – JMy Jul 25 '23 at 18:46
  • I'll try to run without the allocator to see if i can avoid the seg fault – JMy Jul 25 '23 at 18:47
  • 1
    Tested without allocator, seg fault still present. I imagine it has to do with my input from Python. Not sure how to create `std::vector>` from the Python side. I am passing my input to `__call__` in the same manner as I did when the input was of type `std::vector`, this seems wrong. – JMy Jul 25 '23 at 18:57
  • 1
    Please post a [mcve]. – n. m. could be an AI Jul 25 '23 at 19:10
  • I'll make one and share it here @n.m.willseey'allonReddit! Just to have asked: Do you immediately see anything wrong with the dereferencing in the last implementation of `operator_py()`? – JMy Jul 25 '23 at 19:25
  • 2
    I don't see anything wrong, primarily because I don't see anything I can check. I would like to look at something *testable*. A bunch of ellipses and undefined identifiers ain't it. – n. m. could be an AI Jul 25 '23 at 19:52
  • Offcourse! MWE on the way :-) – JMy Jul 25 '23 at 20:01
  • 1
    So I just completed the MWE, and it worked like expected, output: `[, ] [, ]`. Hence the mistake is inside the "processing" body of operator_py(). I'll update when I figure out the mistake with my processing – JMy Jul 25 '23 at 21:03

1 Answers1

3

Thanks to comments from @DanMašek and @n.m.willseey'allonReddit it was unveiled that making these changes to operator_py() fixes the issue in the question:

template <class Params>
std::vector<std::shared_ptr<Obstacle>>& operator_py(
    std::vector<std::shared_ptr<Obstacle>> &obstacles,
    const Params &params
    )
    {
    for (auto& obstacle : obstacles)
        {
             obstacle->someObstacleClassMethod()
             someObstacleProcessMethod(obstacles, params)
        }   
    return obstacles;
    }

With this implemented the desired output was obtained:

import example

obstacle_1 = example.Obstacle()
obstacle_2 = example.Obstacle()

obstacles = [obstacle_1, obstacle_2]
print(obstacles)

params = example.Params()
obstacle_process = example.ObstacleProcess(params)
obstacles = obstacle_process(obstacles)
print(obstacles)

# output:
[<example.Obstacle object at 0x7f709bdc2430>, <example.Obstacle object at 0x7f709b12a830>]
[<example.Obstacle object at 0x7f709bdc2430>, <example.Obstacle object at 0x7f709b12a830>]
JMy
  • 73
  • 8
  • 1
    Glad it helped. I bet you could also get rid of that intermediate vector and marge both loops together. – Dan Mašek Jul 25 '23 at 21:49
  • Yeah, probably! But I'd have to rewrite some stuff as I have some methods which admits `obstacles` and a lot of other stuff which only uses `obstacle` inside the last loop. – JMy Jul 25 '23 at 21:53
  • 1
    Correct again @DanMašek! Removed the unnecessary loop as `obstacles` and `obstacle` should be used directly as shown in the answer. Now the answer is completed (Y). – JMy Jul 26 '23 at 10:26