Parametric improvements #72

S-Dafarra · 2024-02-29T17:05:33Z

Fixes #68
Fixes #69
Fixes #70
Fixes #71

This PR also adds a conversion function from an adam model to an iDynTree model

…rd_kinematics method In the parametric casadi implementation

This needs ami-iit/adam#72 to be merged

To be tested

Using link.inertial.origin instead

…perator

It should be symmetric anyhow

… idyntree conversion

S-Dafarra · 2024-03-08T15:34:32Z

Tests are failing because the changes of robotology/idyntree#1160 are not yet available in conda.

…model

S-Dafarra · 2024-03-11T12:54:49Z

Tests are failing because the changes of robotology/idyntree#1160 are not yet available in conda.

It should be soon conda-forge/idyntree-feedstock#78

S-Dafarra · 2024-03-12T11:23:15Z

CI is now happy. The PR is ready to be reviewed. In particular, I would love the comments of:

@Giulero for the changes in the type hinting
@CarlottaSartore for the changes in the parametric part
@traversaro for the conversion to an iDynTree model

CarlottaSartore · 2024-03-12T11:46:34Z

src/adam/parametric/casadi/computations_parametric.py

@@ -248,28 +250,27 @@ def gravity_term_fun(self) -> cs.Function:
            self.f_opts,
        )

-    def forward_kinematics(self, frame, T_b, s) -> cs.Function:


Adding this only to recall the reason why this has been deleted:
there exists the function forward_kinematics_fun, this was a left over.

src/adam/parametric/casadi/computations_parametric.py

src/adam/parametric/jax/computations_parametric.py

src/adam/parametric/numpy/computations_parametric.py

src/adam/parametric/pytorch/computations_parametric.py

Giulero

Thanks @S-Dafarra, you fixed a lot of old wrong typing!
The idyntree model export is great! We could also use the same logic in the future to create adam models from idyntree ones.
Btw everything looks good to me :)

Giulero · 2024-03-12T14:25:52Z

src/adam/model/conversions/idyntree.py

+    for i in range(3):
+        for j in range(3):
+            idyn_spatial_rotational_inertia.setVal(i, j, inertia_matrix[i, j])


Not really sure about this. Is it possible to use idyntree.fromPython?
Maybe it just works with some specific types.

I did try that, but unfortunately FromPython is defined byMatrix3x3 from which RotationalInertia inherits, and it is a static method, so it returns a Matrix3x3 instead of a RotationalInertia. In the end, this was the only method I managed to use reliably

Yeah, I was imagining it :)

Giulero · 2024-03-12T14:32:51Z

src/adam/model/model.py

-    def get_ordered_link_list(self):
-        """get the ordered list of the link, based on the direction of the graph
-
-        Returns:
-            list: ordered link list
-        """
-        return self.tree.get_ordered_nodes_list()
-


Yeah, a never used method 😄

Yes, it was throwing a warning since self.tree.get_ordered_nodes_list requires an argument that was not passed here, so I figured it was not really used 😊

Giulero · 2024-03-12T14:47:49Z

During the review, I also figured out that some files are signed with the old license (see #64)

# This software may be modified and distributed under the terms of the
# GNU Lesser General Public License v2.1 or any later version.

I will update them in a different PR.

traversaro · 2024-03-14T14:33:20Z

src/adam/model/conversions/idyntree.py

+    Returns:
+        iDynTree.SolidShape: the iDynTree solid shape
+    """
+    visual_position = idyntree.bindings.Position.FromPython(visual.origin.xyz)


I vaguely recall that due to a change by @GiulioRomualdi it is not necessary anymore to call FromPython, but for sure he remember better.

traversaro · 2024-03-14T14:40:21Z

src/adam/model/conversions/idyntree.py

+    for i in range(3):
+        for j in range(3):
+            idyn_spatial_rotational_inertia.setVal(i, j, inertia_matrix[i, j])
+    rotated_inertia = inertia_rotation * idyn_spatial_rotational_inertia


Nice that you avoided this, it is a common bug in URDF parsers, see ros/robot_model#66 and ros/robot_model#117 .

Giulero · 2024-03-21T16:34:03Z

Hi @S-Dafarra ! Can we merge the PR? :)

S-Dafarra · 2024-03-21T16:35:59Z

Hi @S-Dafarra ! Can we merge the PR? :)

Hi @Giulero, there was this point open, but at this point I think we can proceed 😉

traversaro · 2024-03-21T18:34:52Z

Hi @S-Dafarra ! Can we merge the PR? :)

Hi @Giulero, there was this point open, but at this point I think we can proceed 😉

Sure!

Giulero · 2024-03-22T10:40:29Z

Thanks a lot! <3 Merging.

S-Dafarra added 3 commits February 29, 2024 18:00

Added PyCharm conf files to gitignore

b231b38

Workaround issue while loading stickBot model

71f3732

Fixed order of arguments in get_total_mass and removed leftover forwa…

48ae25b

…rd_kinematics method In the parametric casadi implementation

This was referenced Feb 29, 2024

Wrong return type hint in parametric get_total_mass #69

Closed

Failed to load stickBot model in the parametric KinDynComputations. #71

Closed

S-Dafarra added a commit to ami-iit/hippopt that referenced this pull request Mar 4, 2024

Inverting the inputs to get_total_mass

9f57742

This needs ami-iit/adam#72 to be merged

S-Dafarra added 3 commits March 5, 2024 12:34

Fixed typos

d3c8b12

Updating the visual info after the parametric modifications

d87d797

Initial draft to convert an ADAM model to an iDynTree model

430ac4d

S-Dafarra force-pushed the parametric_improvements branch from ba054d0 to 430ac4d Compare March 6, 2024 09:02

S-Dafarra added 19 commits March 6, 2024 11:43

Completed draft of conversion to iDynTree model

bc5f5ee

To be tested

Added initial test for conversion to iDynTree

515bc72

Set visuals

7a92d12

Added handling of frames in the conversion to iDynTree

dc73658

Removed useless aliases in iDynTree conversion

9ddb136

Adding check when setting visuals to avoid CI failures

72e7cb6

Removed useless origin field in Link and added Pose for type hinting

af2d1c4

Removed usages of link origin.

603d997

Using link.inertial.origin instead

Added Inertia type hint and fixed some warnings

8638677

Using List in Pose type hint to avoid warnings about presence of [] o…

55e857d

…perator

Avoiding to use iyx in the inertia.

df57464

It should be symmetric anyhow

Added missing self in method definitions of SpatialMath

a5e21bb

Properly using Pose objects in parametric joint and links

fbeb8e4

Added test for comparison from parametric model to iDynTree

7076329

Added conversion to idyntree to __init__.py

80f54c8

Properly setting the transform of visuals and set joint limits in the…

576584d

… idyntree conversion

Removed check of attribute availability in the conversion to idyntree

4539ebd

Added Limits type hint and fixed other type warnings

e7ee427

Removed unused method with empty start string

66f2273

S-Dafarra added 5 commits March 8, 2024 17:00

Return an error if the input link is not supported in the parametric …

8e25f02

…model

Some type hinting fix in tree.py

fa390b8

Removed useless member in parametric link

fa984b1

Added method to retrieve the original densities

bfb9ffc

Use Union in Tree type hint to be compatible with python 3.8

045922e

This was referenced Mar 8, 2024

Handle the case in which a parametric link is not a box, cylinder or sphere #68

Closed

Allow getting the default densities in the parametric model #70

Closed

Set minimum idyntree version in CI

67a9e7d

S-Dafarra marked this pull request as ready for review March 12, 2024 11:23

S-Dafarra requested review from CarlottaSartore, Giulero and traversaro March 12, 2024 11:23

S-Dafarra self-assigned this Mar 12, 2024

CarlottaSartore approved these changes Mar 12, 2024

View reviewed changes

improved the documentation of the get_original_densities methods

5d2252d

Giulero approved these changes Mar 12, 2024

View reviewed changes

traversaro reviewed Mar 14, 2024

View reviewed changes

Giulero merged commit 45ea57e into ami-iit:main Mar 22, 2024
8 checks passed

S-Dafarra deleted the parametric_improvements branch March 22, 2024 10:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parametric improvements #72

Parametric improvements #72

S-Dafarra commented Feb 29, 2024 •

edited

Loading

S-Dafarra commented Mar 8, 2024

S-Dafarra commented Mar 11, 2024

S-Dafarra commented Mar 12, 2024

CarlottaSartore Mar 12, 2024

Giulero left a comment

Giulero Mar 12, 2024

S-Dafarra Mar 12, 2024

Giulero Mar 12, 2024

Giulero Mar 12, 2024

S-Dafarra Mar 12, 2024

Giulero commented Mar 12, 2024

traversaro Mar 14, 2024

traversaro Mar 14, 2024

Giulero commented Mar 21, 2024

S-Dafarra commented Mar 21, 2024

traversaro commented Mar 21, 2024

Giulero commented Mar 22, 2024

Parametric improvements #72

Parametric improvements #72

Conversation

S-Dafarra commented Feb 29, 2024 • edited Loading

S-Dafarra commented Mar 8, 2024

S-Dafarra commented Mar 11, 2024

S-Dafarra commented Mar 12, 2024

Choose a reason for hiding this comment

Giulero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Giulero commented Mar 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Giulero commented Mar 21, 2024

S-Dafarra commented Mar 21, 2024

traversaro commented Mar 21, 2024

Giulero commented Mar 22, 2024

S-Dafarra commented Feb 29, 2024 •

edited

Loading