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

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

Open
rmcilroy opened this issue Jun 6, 2022 · 3 comments
Open

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

rmcilroy opened this issue Jun 6, 2022 · 3 comments

Comments

@rmcilroy
Copy link

rmcilroy commented Jun 6, 2022

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<value_type>&>(caster));
...

I guess this might be related to std::vector being potentially specialized as a bitset instead of an array of byte sized bools (abseil/abseil-cpp#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
Copy link
Contributor

rwgk commented Jun 6, 2022

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
Copy link
Author

rmcilroy commented Jun 7, 2022

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
Copy link
Contributor

rwgk commented Jun 8, 2022

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>>.

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