-
Notifications
You must be signed in to change notification settings - Fork 67
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
Create function that returns mutiple transforms based on a list of frames #659
Comments
A first dumb implementation could simply use a |
For testing purposes the following funcitons were implemented: std::vector<iDynTree::Transform> getWorldTransforms( std::vector<int> frameIndexes);
std::vector<iDynTree::Transform> getWorldTransformsIndex( std::vector<int> frameIndexes);
std::vector<iDynTree::Transform> getWorldTransforms(std::vector<std::string> frameNames);
std::vector<iDynTree::Matrix4x4> getWorldTransformsAsHomogeneous(std::vector<std::string> frameNames); For testing/benchmarking the following script was used: testGetWorldTransformTimes.txt The lines with ind as legend means get each transform individually in a for loop. The others are in the same order as the mentioned implemented functions. This suggest that any improvements on the C++ side might have negligible impact on the overall timings in Matlab. It can be seen that there seems to be no gain in getting all the transforms except for the function that gives directly the homogenous transform. The reason is probably that even if we get a vector of transforms we still need to iterate through the vector and transform each individual transform to matlab matrix. The reason homogeneous transform seem to have the upper hand is because we save the asHomogeneousTransform step in the bindings. Here we can se why it takes less time to get directly the homogeneous transform: Despite it takes half the time to do just toMatlab, the time decreases by a third since we go through the iterator to ge the individual matrix and then do to Matlab. If we do it in a single line we get a further optimization although small. I doubt it is possible, but what could further help for the visualization in terms of time optimization is to be able to get from a single swig function directly matlab matrices without the need to do the toMatlab call. |
With the changes and the use of the new function we have that the mean of 100 update times are: % using iDyntree string vector
stringVector_time = 0.0018
% using a string cell array with the link names
cellArray_time = 0.0275 It is an order of magnitude difference. Done in 2a846e0 |
It seems that CI is failing. The message is: @traversaro this does not seem related to the changes I did. Could I proceed to open a pull request? Should I open an issue for this error? |
See also #667 (comment). Probably it is enough for you to rebase on master. |
I would have some curiosities:
|
This was fixed by #667 , please rebase and it should work fine. |
It is possible, but you need to inject the C++ code directly in the SWIG generated code, see for example as the toMatlab call itself is implemented: idyntree/bindings/matlab/matlab_matvec.i Line 119 in 114671f
|
It was a test I did when seeing that the time between using indexes and a frame name was pretty much the same. I had the doubt if maybe because of the overloading of the function something was taking a bit of extra time. So I created one that only takes integers, but the result was the same. It was deleted later since it was purely to test that.
My understanding is that we can don't know if it would be more efficient. I can give it a try.
Avoiding the toMatlab is something I also want to try, even without sending the output vector. For the C++ side I think is straight forward if I create a iDynTree.MatrixVector variable ( soon to be renamed to Matrix4x4Vector ). Now using both the sending of the output parameter and avoid using the toMatlab conversion is something that I'm not sure out my head how to do.
I honestly doubt is worth it at the moment. If you see the difference between using the index and the frame name or avoiding doing the homogenousTransform call from matlab, for me is clear that the c++ code runs faster and that the swig conversions are slower. So I would further improve the swig part before like avoiding the toMatlab call. Then if we are still not satisfied with the time we can try to look more deeply at the C++ code.
That is exactly my goal with avoiding the toMatlab call. For now, after I receive the vector of matrices, I need to transform each to a matlab variable, so I enter a for loop converting each from c++ to matlab, for efficiency in the update function I use the same for loop to set the resulting matrix as the transform for the meshes. See updateVisualization |
Found a bug, it was not updating all the transforms just the first. % using iDyntree string vector
stringVector_time = 0.0157
% using a string cell array with the link names
cellArray_time = 0.0241 Still improved but not as dramatically. Fixed in 730ac9f Benchmarking code (to use rename file extension to .m): |
While I was thinking about what could be the cause for the |
Exactly, converting the input to |
Thanks for trying, but in the code it seems you forgot the &
You are still passing by copy 😁 Btw..at what time did you test it? 😲 |
Oops, you are right I think the answer to this is the answer to the next question haha.
Baby woke me up at 4 couldn't go back to sleep so might as well work a bit. Guess I wasn't as awake as I thought. I'll redo the tests and see if it changes. |
It is still valid. I guess reaffirms my idea that is not the C++ code doing the slowing down. |
By the way I found this:
|
Too bad. I agree that the C++ implementation may not change that figure too much, but I guess that at least its interface may have an effect. Regarding the "const" by itself, I was not expecting any difference. Apart from some compiler optimization, it should not change much from the performance point of view. Actually, in the past we noticed that you can modify from Matlab some variables which are not supposed to be modified (they were marked |
Finally, I was able to finish the extra funciton in swig
As seen from the results, the main time consumption now is in the actual The time for the updates becomes: This time we were able to get a x5 times faster update in the visualization. The difference with respect to the previous timings seems to be matlab going inside the for loop to update the transform objects. As can be seen here: So now matlab Added in e398eb9 Scripts used to test: |
PR merged so we can close this. |
In cases where we are updating information outside of iDyntree. Such as the new Matlab visualizer .
The idea for this function is:
cc @S-Dafarra @traversaro
The text was updated successfully, but these errors were encountered: