Extend iDynTree::ModelLoader::loadReducedModelFromFullModel method to optionally take a given value for lumped models

robotology / idyntree

Multibody Dynamics Library designed for Free Floating Robots

BSD 3-Clause "New" or "Revised" License

155 stars 65 forks source link

Extend iDynTree::ModelLoader::loadReducedModelFromFullModel method to optionally take a given value for lumped models #1174

Closed traversaro closed 2 months ago

traversaro commented 2 months ago

Fix https://github.com/robotology/idyntree/issues/495 .

Currently untested, I added functions instead of adding new default values to preserve ABI. Functions can be removed right before a stable release.

GiulioRomualdi commented 2 months ago

Thank you @traversaro this is great!!

traversaro commented 2 months ago

CI failures are unrelated, I did not tested the functions, but if you want to use the functionalities we can merge and do a release once you validated the function.

traversaro commented 2 months ago

I added tests, this is ready for review.

traversaro commented 2 months ago

Great, only macos test fail with:

24/68 Test #24: UnitTestModel .........................................Subprocess aborted***Exception:   0.01 sec
Checking simple model... 
Checking random chains...
Checking reduced model for random chain of size: 2
Checking reduced model for random chain of size: 17

valgrind tests on Linux work fine. @GiulioRomualdi realistically I can't debug this until next week as I do not hav a mac. We can disable the tests on mac and release, or start testing the method from this branch, let me know what you prefer.

traversaro commented 2 months ago

Great, only macos test fail with:
24/68 Test #24: UnitTestModel .........................................Subprocess aborted***Exception:   0.01 sec
Checking simple model... 
Checking random chains...
Checking reduced model for random chain of size: 2
Checking reduced model for random chain of size: 17
valgrind tests on Linux work fine. @GiulioRomualdi realistically I can't debug this until next week as I do not hav a mac. We can disable the tests on mac and release, or start testing the method from this branch, let me know what you prefer.

Note that I noticed that no one was calling the checkReducedModel test and I added it in tests now, so I guess that could also be a macos failure that was always there, and nothing something introduced by this PR.

traversaro commented 2 months ago

Note that I noticed that no one was calling the checkReducedModel test and I added it in tests now, so I guess that could also be a macos failure that was always there, and nothing something introduced by this PR.

Actually not: https://github.com/robotology/idyntree/pull/1176 .

traversaro commented 2 months ago

Great, only macos test fail with:
24/68 Test #24: UnitTestModel .........................................Subprocess aborted***Exception:   0.01 sec
Checking simple model... 
Checking random chains...
Checking reduced model for random chain of size: 2
Checking reduced model for random chain of size: 17
valgrind tests on Linux work fine. @GiulioRomualdi realistically I can't debug this until next week as I do not hav a mac. We can disable the tests on mac and release, or start testing the method from this branch, let me know what you prefer.

There was a bug in the test function, that was setting some position value even if the joint had 0 DOF, I fixed it with the check in https://github.com/robotology/idyntree/pull/1174/files#diff-f8a68ac51cda85238110c1087da7b9715dd2e0d44e16913fd07217c8c43d24e8R106 .