pybind / pybind11_abseil

Pybind11 bindings for the Abseil C++ Common Libraries
Other
23 stars 13 forks source link

Error when converting absl::Span<const bool> #4

Open rmcilroy opened 2 years ago

rmcilroy commented 2 years ago

When trying to specify a py::arg with C++ type absl::Span (or more specifically in my case absl::optional<absl::Span>), I get an compile error:

./third_party/pybind11_abseil/absl_casters.h:378:12: error: no matching function for call to 'MakeSpan' return absl::MakeSpan(static_cast<std::vector&>(caster)); ...

I guess this might be related to std::vector being potentially specialized as a bitset instead of an array of byte sized bools (https://github.com/abseil/abseil-cpp/issues/644). It would be nice to be able to define an py::arg as absl::Span though if it's possible to side-step this issue.

rwgk commented 2 years ago

Did you already try solving this with a lambda? That would be a good first step, and a good starting point for thinking about a general solution.

rmcilroy commented 2 years ago

The issue is the lack of a ".data()" method on the std::vector specialization, which causes absl::MakeSpan fail to compile when passed a std::vector. I can make the code compile if I pass a lambda that uses a different container type (e.g., const absl::InlinedVector<bool, 4>) as the argument, then converts this to a span using absl::MakeSpan before calling the underlying function that takes a Span. However this fails at runtime due to a TypeError in pybind11, presumably since there are no pybind11 convertors from Python's Sequence[bool] type to the C++ absl::InlinedVector<bool, 4> type:

TypeError: ConvGeneralDilated(): incompatible function arguments. The following argument types are supported:

  1. (lhs: google3.third_party.tensorflow.compiler.xla.python.xla_extension.XlaOp, rhs: google3.third_party.tensorflow.compiler.xla.python.xla_extension.XlaOp, window_strides: Span[int], padding: Span[Tuple[int, int]], lhs_dilation: Span[int], rhs_dilation: Span[int], dimension_numbers: xla::ConvolutionDimensionNumbers, feature_group_count: int = 1, batch_group_count: int = 1, precision_config: xla::PrecisionConfig = None, preferred_element_type: Optional[google3.third_party.tensorflow.compiler.xla.python.xla_extension.PrimitiveType] = None, window_reversal: Optional[absl::InlinedVector<bool, 4ul, std::__u::allocator>] = None) -> google3.third_party.tensorflow.compiler.xla.python.xla_extension.XlaOp

Invoked with: <google3.third_party.tensorflow.compiler.xla.python.xla_extension.XlaOp object at 0x7fa600807430>, <google3.third_party.tensorflow.compiler.xla.python.xla_extension.XlaOp object at 0x7fa600807df0>, [1, 1], [(1, 0), (0, 1)], (2, 1), (1, 1), <google3.third_party.tensorflow.compiler.xla.python.xla_client.ConvolutionDimensionNumbers object at 0x7fa5f9482890>; kwargs: window_reversal=[False, True]

Did you forget to #include <pybind11/stl.h>? Or <pybind11/complex.h>, <pybind11/functional.h>, <pybind11/chrono.h>, etc. Some automatic conversions are optional and require extra headers to be included when compiling your pybind11 module.

Does pybind11 support any different C++ container types we could use here instead of std::vector?

rwgk commented 2 years ago

Wow, that's a lot of arguments, difficult to tell, but your guess makes sense.

We have py::sequence:

https://github.com/pybind/pybind11/blob/e2dcd95407d5202019cecd2bb2827ee6a4a8f9f3/tests/test_sequences_and_iterators.cpp#L527

You could iterate over the sequence, which will give you py::handle (or py::object, not sure), which you can then obj.cast<bool>() to .push_back() to a container, which you can then pass via absl::Span. Roughly speaking. Not pretty, but I'm almost certain you can make it work with that general approach.

Much better would be to add type_caster<absl:: InlinedVector<T>> to pybind11_abseil, but that requires more background. Iow that would be a non-trivial investment into a general tooling improvement, while the lambda approach is a quick local workaround.

Yet another idea is to fix up pybind11/stl.h to work for std::vector<bool>, with a fully-specialized type_caster<std::vector<bool>>.