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

Avoid to compute the pseudoinverse to evaluate the external wrenches #1017

Merged
merged 4 commits into from
Aug 30, 2022

Conversation

GiulioRomualdi
Copy link
Member

@GiulioRomualdi GiulioRomualdi commented Aug 26, 2022

@traversaro
Copy link
Member

traversaro commented Aug 27, 2022

I just realized that in most cases we can also avoid the Qr decomposition, as in the submodels has only one full wrench as unknown, in the equation Ax = b the A is just an adjoint matrix for which the inverse is trivial. However, computing this "special case" is probably more complex that this change, so let's see how time the use of Qr saves.

@GiulioRomualdi
Copy link
Member Author

I would like to present you some results. I used perf to profile the new implementation with respect to the old one (the one with pinv)
The command I used is sudo perf record --call-graph dwarf -p <PID> -- sleep 3 then I analyzed the data with hotspot

The following figures shows the result in case of pinv (the old method) and the qr decomposition (what the PR is proposing)

pinv QR decomposition
Screenshot from 2022-08-29 11-37-48 Screenshot from 2022-08-29 11-37-37

Accordingly to the results the approach I proposed is 2 times faster than the original one.

@traversaro
Copy link
Member

Great, let's merge then. Given that the base branch of the PR for some reason is quite old, can I rebase on top of the latest master to run the CI?

@GiulioRomualdi
Copy link
Member Author

Great, let's merge then. Given that the base branch of the PR for some reason is quite old, can I rebase on top of the latest master to run the CI?

sure!

@traversaro traversaro force-pushed the improve_estimate_extenal_wrench branch from 1755551 to a3d8fdc Compare August 29, 2022 12:45
@traversaro
Copy link
Member

traversaro commented Aug 29, 2022

CI is failing as we modified a structures without re-generating MATLAB bindings, I just launched the action to regenerated the bindings: https://github.com/robotology/idyntree/actions/runs/2949426070 .

@GiulioRomualdi
Copy link
Member Author

cc @gabrielenava

@traversaro traversaro merged commit 9e1cb11 into master Aug 30, 2022
@traversaro traversaro deleted the improve_estimate_extenal_wrench branch August 30, 2022 10:31
@DanielePucci
Copy link
Collaborator

@gabrielenava we might consider to re-install the iDyntree for the iRonCub

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

Successfully merging this pull request may close these issues.

3 participants