-
Notifications
You must be signed in to change notification settings - Fork 70
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
Added support to Jacobians sparsity pattern #386
Added support to Jacobians sparsity pattern #386
Conversation
I don't know if we have a changelog in iDynTree. In that case, this is the changelog text HighLevel
- Added method to obtain relative Jacobians sparsity pattern
- Added method to obtain free floating Jacobians sparsity pattern
Inverse Kinematics
- Constraints Jacobian now exploit sparsity pattern |
I tested it for walking and seems ok. The performances did not increase too much, but it is definitely a starting point 😉 |
cd48492
to
4b2ad55
Compare
The changelog is in https://github.com/robotology/idyntree/blob/devel/doc/releases/v0_10.md , it follows the same format of YARP, thanks! |
This remained in the backlog a bit to much, sorry. : ( @S-Dafarra (or @GiulioRomualdi if he want to get his hands in the IK stuff) can you take care of fixing the conflicts? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok modulo fixing the conflicts.
I can deal with this. @GiulioRomualdi tell me if you would have preferred to get this. |
9014510
to
926b5b6
Compare
Done. @francesco-romano if you could roughly check whether all remained the same after the rebase. The memcheck test seems to be much slower. It took 425 seconds in my PC in debug (no failures anyway). |
You mean |
That took me 93 sec. I was meaning |
LGTM Just check the CMakeLists.txt |
Numbers on my PC:
|
Actual tests numbers (the numbers only account for the
|
An increase of the test time may be due to the fact that this PR adds an additional test which inside call the |
Actually I am mistaken, this does not modify the IK unit test. So I have a bit more troubles in understanding why is so different. |
I think it is quite different due to the use of several Eigen methods, that in Debug mode they insert a lot of methods and for some reason run slower in the valgrind virtual machine. The actual numbers seems to indicate that |
Note: the terrible |
Note: the Valgrind failures are due to #456 . |
c2393bf
to
0a6afe6
Compare
Tests on the IK are failing (both the memcheck and the non-memecheck) |
I am not able to reproduce the issue. It works on my PC, both on release and debug (the latter also with valgrind). |
The same for me, but it is strange that devel passes, and this branch (rebased on devel) not |
Also appveyor does not complain. The assertion that fails is on the output flag of the |
I want to work on #458 this week, so we can have consistency between the system that most users use and the CI. After that, we can see if the failure persist. |
The sparsity pattern is added only if the cost/constraint is active.
Discussing with @pattacini and @S-Dafarra we decided to add a check in IpOpt (running only when the code is compiled in debug). We now check that a callback function is called only once for a specified set of input parameters. If this does not happen, the code is still correct, but we execute some code twice. As it has been tested now, this does not happen, so the code does not need any change at this time.
0a6afe6
to
1b8c86c
Compare
Rebased over |
After #458 the CI seems to work. If you are ok with that @S-Dafarra I would merge. |
Probably the release notes need to be moved from |
[ci skip]
Thanks @S-Dafarra . |
Added two methods to
KinDynComputations
to output the Jacobian sparsity pattern.The supported Jacobians are the relative Jacobian and the free floating Jacobian.
The last available Jacobian, i.e. the CoM Jacobian, does not have a sparsity pattern as it is dense by definition.
Adapted IK to use the sparsity pattern for the Jacobians.
A small performance increase (up to 20%) can be notice in some tests.
Of course the more sparse is the Jacobian, the higher the performance benefit.
Potentially, if the Jacobians are dense, it is possible that IK is slightly slower, given the overhead introduced.