-
Notifications
You must be signed in to change notification settings - Fork 768
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
Matlab Wrapper function to extract Vectors from a Values object #733
Conversation
gtsam/nonlinear/utilities.h
Outdated
@@ -142,6 +143,22 @@ Matrix extractPose3(const Values& values) { | |||
return result; | |||
} | |||
|
|||
/// Extract all Vector values into a single tall vector | |||
Vector extractVector(const Values& values) { |
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.
I think an API that makes more sense is to give a character, say 'x`' that will be used to select all values with a particular Symbol, e.g. x4, x5, x6, and that will assume that they are all of the same dimension. Use case is then:
result = extractVectors(values, 'x')
and if there are 200 such vectors, and they are 5D, the result is a 200x5 Matrix (not 5x200, we want to be consistent with other methods in this file). You can thrown an exception if those values have different dimensions.
Also, merge in develop and resolve conflicts (carefully!) |
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.
I love that we are reviving this PR. Just change the docs etc to read mxn
not ‘NxM’, because:
- matrices are typically mxn
- N is a better constant for dimensionality IMHO
I am going to verify this update on my computer now. |
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.
@gchenfc I just switched to this branch, compiled and tried running the testUtilities.m
and immediately ran into:
>> testUtilities
Error using gtsam_wrapper
Exception from gtsam:
eliminateMultifrontal expects 1 arguments, not 0
Error in gtsam.utilities.extractVector (line 3)
varargout{1} = gtsam_wrapper(2379, varargin{:});
Error in testUtilities (line 54)
actual = utilities.extractVector(values);
I am guessing that this may be related to #954 (comment) and to your current wrapper issue.
Oops I had a typo in unit test, can you try again with updated .m file? No clue why EliminateMultifrontal is involved - but if it's still having issues after updated .m file we can chat to resolve. |
I looked at the file changes you made and implemented them on my system so I could quickly test your fix without recompiling and I found the following error:
Looking at the output of the wrapper definition I see the following:
So I tried the following tests:
Am I inputting the symbol correctly? |
Omg I'm so careless - it should be the last one:
Actually the test should be like this:
Sorry for the confusion |
Running this snippet of code resulted in:
This is the correct format and executed flawlessly. |
Can you push these changes so I can pull and then just test the .m files real quick? |
pushed - thanks so much for testing for me!!! |
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.
pushed - thanks so much for testing for me!!!
I have pulled and verified this fix.
>> testUtilities
Thank you for doing this @gchenfc
@mattking-smith and I added a function in
utilities.h
, which is primarily used for the matlab wrapper, to extract all the vectors from a Values object.Similar to extractPose3 and extractPose2, this just makes a lot of matlab code significantly faster than for-looping.