idyntree
idyntree copied to clipboard
robotology
Implement iDynTree base type `type_caster` for pybind11 bindings
In the past few days, I discussed with @traversaro about the possibility to install also pybind11 bindings with the superbuild.
This will simplify the usage of iDynTree bindings along with bipedal-locomotion-framework. In the end, we can use the same approach of manifpy https://github.com/ami-iit/bipedal-locomotion-framework/pull/238#issuecomment-799435166
Given that it would be nice to automatically convert iDynTree base types (VectorDynSize, VectorFixSize, MatrixDynSize and MatrixFixSize) to NumPy objects. Accordingly to pybind11 documentation this can be done implement a custom type_caster.
This PR introduces the machinery to automatically convert iDynTree matrices and vectors to numpy.
Furthermore, this PR introduces:
- The raw buffer constructor for the Position class. This is required to implement the
type_caster - Make
rotationandpositionproperties readable and writable in the pybind11 Transform bindings - The rotation matrix is not automatically converted to NumPy since I would like to avoid the user setting a random matrix that is not a rotation one. Furthermore, if it is converted into a NumPy matrix when the inverse is computed
pythonwill not exploit the rotation matrix properties (inverse == transpose)
Thanks to this PR we can do something like
| Before | Now |
|
|
|
|
TODO
- [x] Update the tests
- [x] Update the CHANGELOG
cc @Giulero
Awesome work @GiulioRomualdi, I'd love to see idyntree pybind11 bindings on par with those autogenerated by SWIG. As we understood in this last period, it's a very effective way to implement interoperability between different projects (modulo some edge case https://github.com/ami-iit/bipedal-locomotion-framework/issues/386).
I put this PR in my review list, I also add @francesco-romano as reviewer since he was the original author of the pybind11 version of the bindings.
The PR is ready to be reviewed. I'm pretty sure that it is possible to improve the performances of the type_caster functions since NumPy and iDynTree store the matrices in row-major mode. But I am not an expert of that
The change seems to be nice, however from what I understand it is a breaking change, right? I mean, now the code:
material = iDynTree.Material()
color = iDynTree.Vector4()
color[0] = 1.0
color[1] = 0.0
color[2] = 0.0
color[3] = 1.0
material.color = color
is not working anymore?
It may be worth to clearly document this in the CHANGELOG then. I would also want to make sure that this is ok for @francesco-romano that originally contributed the pybind11 bindings, and I guess is mantaining an internal use of iDynTree/pybind11 code that he may need to migrate. To clarify, this will only be released as iDynTree 5, iDynTree releases <= 4 will keep the non-numpy API.
The change seems to be nice, however from what I understand it is a breaking change, right? I mean, now the code:
material = iDynTree.Material() color = iDynTree.Vector4() color[0] = 1.0 color[1] = 0.0 color[2] = 0.0 color[3] = 1.0 material.color = coloris not working anymore?
It may be worth to clearly document this in the CHANGELOG then. I would also want to make sure that this is ok for @francesco-romano that originally contributed the pybind11 bindings, and I guess is mantaining an internal use of iDynTree/pybind11 code that he may need to migrate. To clarify, this will only be released as iDynTree 5, iDynTree releases <= 4 will keep the non-numpy API.
Yes, this is an important topic that we should discuss. iDynTree.Vector4 does not exist anymore. Indeed iDynTree.Vector4 == np.array The same is valid for all the other types.
Indeed if a vector of type Type satisfies is_idyntree_vector_fix_size<Type>::value the following type_caster is used
template <typename Type>
struct type_caster<Type, enable_if_t<is_idyntree_vector_fix_size<Type>::value>>
This struct has two methods, load and cast. The load is used to automatically convert a python default constructible pybind11::array_t into a C++ Type vector. On the other hand cast convert a C++ Type vector into a Python np vector.
For example, given the following c++ class
class Foo
{
void setVector(iDynTree::VectorFixSize<3>& v);
iDynTree::VectorFixSize<3> getVector();
}
and the associated pybind11 code
PYBIND11_MODULE(example, m) {
py::class_<Foo>(m, "Foo")
.def("set_vector", &Foo::setVector)
.def("get_vector", &Foo::getVector);
when you call
foo = Foo()
foo.set_vector([1,2,3])
The python list is [1,2,3] is converted by pybind into a pybind11::array_t then since the following type_caster has been implemented
template <typename Type>
struct type_caster<Type, enable_if_t<is_idyntree_vector_fix_size<Type>::value>>
pybind knows how to convert a pybind11::array_t into a iDynTree::VectorFixSize<3> using the load method
The same approach is valid for the get_vector() python method
@traversaro, Let me know if it is more clear know
Before going into details in the review / testing the changes, I would like to understand better if this is what we really want or not.
Focusing on the vector / matrices class only, the change will, in practice, remove those classed from Python.
On one side I see the advantage of this. On the other side, we lose explicit typing. See this as implicit vs explicit conversion.
Are we ok with this?
(the alternative approach, to be tested, is to add conversion functions into the vector classes, something like
.def(py::init([](py::buffer const b) {
py::buffer_info info = b.request();
if (info.format != py::format_descriptor<double>::format()
|| info.ndim != 1
|| info.shape[0] != size)
throw std::runtime_error("Incompatible buffer format!");
VectorFixSize<size> v;
memcpy(v.data(), info.ptr, sizeof(double) * size);
return v;
}));
A second question, related to above: If we are ok with implicit conversion. how do we decide Fixed vs Dynamic version when casting? (I am referring to overloads taking both versions). We do not have control on this anymore. (Maybe we could probably order the cast to try Fixed first?)
Third: we would probably lose the ability to resize in-place vectors and matrices. Is this something that might be useful? It is definitely in C++, not sure in Python?
On the other side, we lose explicit typing. See this as implicit vs explicit conversion. Are we ok with this?
I think that the evaluation done by @GiulioRomualdi is that we sacrifice explicit typing to the altar of convenience of numpy, but perhaps @GiulioRomualdi can elaborate more. I am not a big user of Python bindings so I would trust users on the choice of this tradeoff.
A second question, related to above: If we are ok with implicit conversion. how do we decide Fixed vs Dynamic version when casting? (I am referring to overloads taking both versions). We do not have control on this anymore. (Maybe we could probably order the cast to try Fixed first?)
Not sure on this, what is happening in the version proposed by this PR @GiulioRomualdi ?
Third: we would probably lose the ability to resize in-place vectors and matrices. Is this something that might be useful? It is definitely in C++, not sure in Python?
I guess also this is something that we sacrifice for using natively numpy.
Hi, @francesco-romano and @traversaro sorry for replying late.
On the other side, we lose explicit typing. See this as implicit vs explicit conversion. Are we ok with this?
I think that the evaluation done by @GiulioRomualdi is that we sacrifice explicit typing to the altar of convenience of numpy, but perhaps @GiulioRomualdi can elaborate more. I am not a big user of Python bindings so I would trust users on the choice of this tradeoff.
The idea is exactly this one, we could use directly numpy arrays instead of iDynTree one.
A second question, related to above: If we are ok with implicit conversion. how do we decide Fixed vs Dynamic version when casting? (I am referring to overloads taking both versions). We do not have control on this anymore. (Maybe we could probably order the cast to try Fixed first?)
Not sure on this, what is happening in the version proposed by this PR @GiulioRomualdi ?
In this case, if you try to call a function that takes as input a FixedSizeVector with a NumPy vector of the wrong size you will get an error.
Third: we would probably lose the ability to resize in-place vectors and matrices. Is this something that might be useful? It is definitely in C++, not sure in Python?
I guess also this is something that we sacrifice for using natively numpy.
This conversion is exactly the same as what is happening with the eigen vectors in pybind11. An eigen vector becomes a NumPy array. So you cannot call anymore the methods of the eigen vectors because they are actually numpy arrays. Here it is exactly the same
Sounds good on everything. Just one last question here:
A second question, related to above: If we are ok with implicit conversion. how do we decide Fixed vs Dynamic version when casting? (I am referring to overloads taking both versions). We do not have control on this anymore. (Maybe we could probably order the cast to try Fixed first?)
Not sure on this, what is happening in the version proposed by this PR @GiulioRomualdi ?
In this case, if you try to call a function that takes as input a
FixedSizeVectorwith a NumPy vector of the wrong size you will get an error.
Maybe it does not matter at all but my question is the following. What if you have the following method:
void Foo(iDynTree::VectorDynSize& v) {
// Do something with the vector
}
template <int size N>
void Foo(iDynTree::VectorFixedSize<N>& v) {
// Very optimised method!
}
Which of the two is taken from Python?
Sounds good on everything. Just one last question here:
A second question, related to above: If we are ok with implicit conversion. how do we decide Fixed vs Dynamic version when casting? (I am referring to overloads taking both versions). We do not have control on this anymore. (Maybe we could probably order the cast to try Fixed first?)
Not sure on this, what is happening in the version proposed by this PR @GiulioRomualdi ?
In this case, if you try to call a function that takes as input a
FixedSizeVectorwith a NumPy vector of the wrong size you will get an error.Maybe it does not matter at all but my question is the following. What if you have the following method:
void Foo(iDynTree::VectorDynSize& v) { // Do something with the vector } template <int size N> void Foo(iDynTree::VectorFixedSize<N>& v) { // Very optimised method! }Which of the two is taken from Python?
Regarding this I can write a simple test, by the way I think the easiest way in this case is to create two python functions that have different names
Hi @francesco-romano, this is the outcome
void foo(iDynTree::VectorDynSize&v)
{
std::cout << "vectordynsize"<< std::endl;
}
void foo(iDynTree::VectorFixSize<4>& v)
{
std::cout << "vectorfixedsize"<< std::endl;
}
namespace py = ::pybind11;
PYBIND11_MODULE(pybind, m) {
m.def("foo", py::overload_cast<VectorFixSize<4>&>(::foo))
.def("foo", py::overload_cast<VectorDynSize&>(::foo));
}
import idyntree.pybind as idyn
In [2]: idyn.foo([1,1,1])
vectordynsize
In [3]: idyn.foo([1,1,1,1])
vectorfixedsize
However, I noticed that the order in the definition of the bindings matters. Indeed if the order is the following one
m.def("foo", py::overload_cast<VectorDynSize&>(::foo))
.def("foo", py::overload_cast<VectorFixSize<4>&>(::foo));
I got
In [1]: import idyntree.pybind as idyn
In [2]: idyn.foo([1,1,1])
vectordynsize
In [3]: idyn.foo([1,1,1,1])
vectordynsize
So in conclusion I would define two different python function such as
m.def("foo_vectordynsize", py::overload_cast<VectorDynSize&>(::foo))
.def("foo_vectorfixsize", py::overload_cast<VectorFixSize<4>&>(::foo));
to avoid confusion
Thanks for the test. I think this should be good enough. Let me start the proper review :)
I tried to understand how to write a test for the type_caster and this is an example: https://github.com/pybind/pybind11/blob/dac74ebdf5d64cb30a4492003a87d7455ad00144/tests/test_custom_type_casters.cpp
However, before implementing it I think I should find an answer to this question: https://github.com/robotology/idyntree/pull/931#discussion_r754967060. Indeed it is not clear to me whats the role of convert parameter in the load method and how to implement the different return type policies. Indeed currently the type_caster returns always a copy of the object. Is this wanted?
I tried to understand how to write a test for the
type_casterand this is an example: https://github.com/pybind/pybind11/blob/dac74ebdf5d64cb30a4492003a87d7455ad00144/tests/test_custom_type_casters.cpp
I am unsure if you actually need that. I would have probably created a custom binding for a test, with a class with a constructor with a vector/matrix and then exposing some methods to query the iDynTree object. But maybe you find something smarter in that example
However, before implementing it I think I should find an answer to this question: #931 (comment). Indeed it is not clear to me whats the role of
convertparameter in theloadmethod and how to implement the different return type policies. Indeed currently thetype_casterreturns always a copy of the object. Is this wanted?
Sorry I might have messed up with the comments. The link does not open anything to me. What are you referring to?
The content of this comment has been fixed with https://github.com/robotology/idyntree/pull/931/commits/f04e844802f585d05db557db335dc107b5c26b38
Given the new implementation of the type_caster the tests fail with the following errors
Indeed, since the matrices and vector type are no more defined as iDynTree objects all the classes that inherit from them do not have the access methods that were available before (for instance the operator() is no more defined)
How should we approach the problem?
63: Test command: /usr/bin/python3.8 "-m" "unittest" "discover" "-s" "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests"
63: Test timeout computed to be: 1500
63: EEEEEEEEEEEEEEEEEEEE.E.EE.............E.....E..E.....E..E............E...
63: ======================================================================
63: ERROR: test_read_origin_and_direction_properties (test_idyntree_core.IDynTreeAxisTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 199, in test_read_origin_and_direction_properties
63: self.assertEqual(list(axis.direction), raw_dir)
63: TypeError: 'idyntree.pybind.Direction' object is not iterable
63:
63: ======================================================================
63: ERROR: test_creation (test_idyntree_core.IDynTreeDirectionTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 188, in test_creation
63: self.assertEqual(list(direction), raw_dir)
63: TypeError: 'idyntree.pybind.Direction' object is not iterable
63:
63: ======================================================================
63: ERROR: test_add_operation (test_idyntree_core.IDynTreePositionTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 36, in test_add_operation
63: self.assertEqual(sum_pos[0], 5)
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_negate_operator (test_idyntree_core.IDynTreePositionTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 26, in test_negate_operator
63: self.assertEqual(neg_pos[0], -1)
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_non_zero_creation (test_idyntree_core.IDynTreePositionTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 19, in test_non_zero_creation
63: self.assertEqual(pos[0], 1)
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_subtract_operation (test_idyntree_core.IDynTreePositionTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 47, in test_subtract_operation
63: self.assertEqual(sum_pos[0], -3)
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_zero_position (test_idyntree_core.IDynTreePositionTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 13, in test_zero_position
63: self.assertEqual(len(pos), 3)
63: TypeError: object of type 'idyntree.pybind.Position' has no len()
63:
63: ======================================================================
63: ERROR: test_composition (test_idyntree_core.IDynTreeRotationTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 86, in test_composition
63: self.assertEqual(r_final[r, c], r_expected[r, c])
63: TypeError: 'idyntree.pybind.Rotation' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_create_identity (test_idyntree_core.IDynTreeRotationTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 61, in test_create_identity
63: self.assertEqual(rotation[r, c], 1)
63: TypeError: 'idyntree.pybind.Rotation' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_explicit_constructor (test_idyntree_core.IDynTreeRotationTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 72, in test_explicit_constructor
63: self.assertEqual(rotation[r, c], rot_raw[r, c])
63: TypeError: 'idyntree.pybind.Rotation' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_inverse (test_idyntree_core.IDynTreeRotationTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 98, in test_inverse
63: self.assertAlmostEqual(rot_inv[r, c], exp_rot[r, c])
63: TypeError: 'idyntree.pybind.Rotation' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_position_rotation (test_idyntree_core.IDynTreeRotationTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 108, in test_position_rotation
63: self.assertAlmostEqual(pos_rotated[i], expected_pos[i])
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_creation (test_idyntree_core.IDynTreeSpatialInertiaTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 209, in test_creation
63: rotational_inertia[1, 1] = 1
63: TypeError: 'idyntree.pybind.RotationalInertia' object does not support item assignment
63:
63: ======================================================================
63: ERROR: test_sum (test_idyntree_core.IDynTreeSpatialInertiaTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 245, in test_sum
63: rotational_inertia[1, 1] = 1
63: TypeError: 'idyntree.pybind.RotationalInertia' object does not support item assignment
63:
63: ======================================================================
63: ERROR: test_sum_assignment (test_idyntree_core.IDynTreeSpatialInertiaTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 281, in test_sum_assignment
63: rotational_inertia[1, 1] = 1
63: TypeError: 'idyntree.pybind.RotationalInertia' object does not support item assignment
63:
63: ======================================================================
63: ERROR: test_creation (test_idyntree_core.IDynTreeTransformTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 122, in test_creation
63: self.assertEqual(transform.rotation[r, c], rotation[r, c])
63: TypeError: 'idyntree.pybind.Rotation' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_creation_from_numpy (test_idyntree_core.IDynTreeTransformTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 128, in test_creation_from_numpy
63: rotation = np.array([0, 0, 1],
63: TypeError: array() takes from 1 to 2 positional arguments but 3 were given
63:
63: ======================================================================
63: ERROR: test_identity (test_idyntree_core.IDynTreeTransformTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 142, in test_identity
63: self.assertEqual(transform.position[i], 0)
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_position_transform (test_idyntree_core.IDynTreeTransformTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 159, in test_position_transform
63: self.assertAlmostEqual(pos_transformed[i], expected_pos[i])
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_transform_composition (test_idyntree_core.IDynTreeTransformTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_core.py", line 176, in test_transform_composition
63: self.assertEqual(t_final.position[i], exp_position[i])
63: TypeError: 'idyntree.pybind.Position' object is not subscriptable
63:
63: ======================================================================
63: ERROR: test_rest_transform (test_idyntree_model.IDynTreeFixedJointTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 207, in test_rest_transform
63: self.assertEqual(list(joint_transform.position), list(position))
63: TypeError: 'idyntree.pybind.Position' object is not iterable
63:
63: ======================================================================
63: ERROR: test_inertia (test_idyntree_model.IDynTreeLinkTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 112, in test_inertia
63: rotational_inertia[1, 1] = 1
63: TypeError: 'idyntree.pybind.RotationalInertia' object does not support item assignment
63:
63: ======================================================================
63: ERROR: test_color (test_idyntree_model.IDynTreeMaterialTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 21, in test_color
63: self.assertEqual(material.color, color)
63: File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
63: assertion_func(first, second, msg=msg)
63: File "/usr/lib/python3.8/unittest/case.py", line 902, in _baseAssertEqual
63: if not first == second:
63: ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
63:
63: ======================================================================
63: ERROR: test_model_frame (test_idyntree_model.IDynTreeModelTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 368, in test_model_frame
63: self.assertEqual(list(frame_transform.position), list(position))
63: TypeError: 'idyntree.pybind.Position' object is not iterable
63:
63: ======================================================================
63: ERROR: test_rest_transform (test_idyntree_model.IDynTreePrismaticJointTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 173, in test_rest_transform
63: self.assertEqual(list(joint_transform.position), list(position))
63: TypeError: 'idyntree.pybind.Position' object is not iterable
63:
63: ======================================================================
63: ERROR: test_rest_transform (test_idyntree_model.IDynTreeRevoluteJointTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 139, in test_rest_transform
63: self.assertEqual(list(joint_transform.position), list(position))
63: TypeError: 'idyntree.pybind.Position' object is not iterable
63:
63: ======================================================================
63: ERROR: test_mesh_scale (test_idyntree_model.IDynTreeSolidShapesTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 71, in test_mesh_scale
63: self.assertEqual(mesh.scale, scale)
63: File "/usr/lib/python3.8/unittest/case.py", line 912, in assertEqual
63: assertion_func(first, second, msg=msg)
63: File "/usr/lib/python3.8/unittest/case.py", line 902, in _baseAssertEqual
63: if not first == second:
63: ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
63:
63: ======================================================================
63: ERROR: test_transform (test_idyntree_model.IDynTreeSolidShapesTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_model.py", line 45, in test_transform
63: self.assertEqual(list(mesh.link_H_geometry.position), list(position))
63: TypeError: 'idyntree.pybind.Position' object is not iterable
63:
63: ======================================================================
63: ERROR: test_link_sensor_transform (test_idyntree_sensors.IDynTreeSensorsTest)
63: ----------------------------------------------------------------------
63: Traceback (most recent call last):
63: File "/home/gromualdi/robot-code/robotology-superbuild/src/iDynTree/bindings/pybind11/tests/test_idyntree_sensors.py", line 59, in test_link_sensor_transform
63: sensor.link_sensor_transform.rotation[r, c], rotation[r, c])
63: TypeError: 'idyntree.pybind.Rotation' object is not subscriptable
63:
63: ----------------------------------------------------------------------
63: Ran 73 tests in 0.004s
63:
63: FAILED (errors=29)
1/1 Test #63: pybind11_idyntree_test ...........***Failed 0.16 sec
I implemented the tests and I fixed some bugs in the type_caster. Let me know if I should do something else
@GiulioRomualdi sorry, devel was not updated with the latest CI fix. Feel free to merge again now, thanks!
Sorry for jumping in so late but this PR stayed in my backlog for too long. Thanks @francesco-romano for the thorough review, and @GiulioRomualdi for iterating over it. I do not currently have any downstream code that uses the previous version of the pybind11 bindings, so I fully trust the judgement of @francesco-romano since he is the (only?) user -and original author- of these bindings version, and for sure he's the person with most interest in preserving compatibility / minimizing changes.
I went through you comments, and yes, I confirm that in the Python side, when there are multiple binded methods / functions that could cast the same time, their C++ definition order matters as you found out from your tests.
What's not 100% clear to me without directly testing the code of this PR is how memory ownership is dealt. The default Eigen / NumPy conversion has two different modalities for input data: pass-by-value that always involves a copy, and pass-by-reference that shares the memory. Instead, for output data, this is the relevant documentation.
@GiulioRomualdi, can you please comment about what this PR currently implements? At this first stage, I would not mind on optimizing performances by reducing copies, it can be done in the future if not yet supported, I'd like just to understand what to expect especially from the data returned to Python from the bindings.
Also, storage order (column-major vs row-major) is another advanced feature to double check.
@GiulioRomualdi, can you please comment about what this PR currently implements? At this first stage, I would not mind on optimizing performances by reducing copies, it can be done in the future if not yet supported, I'd like just to understand what to expect especially from the data returned to Python from the bindings.
Only the copy is implemented
Also, storage order (column-major vs row-major) is another advanced feature to double check.
iDynTree stores the matrices in row-major like numpy so it shouldn't be a problem
Also, storage order (column-major vs row-major) is another advanced feature to double check.
iDynTree stores the matrices in row-major like numpy so it shouldn't be a problem
To be precise, this
py::array_t<double, py::array::c_style
is where you specify the row-major ordering. So you are correctly setting it as row-major.
I will need to go over the PR again next week. I lost track of what is pending or not (no longer used to GitHub sorry )
We can close since #1037 implements already one of the key feature designed by this PR