-
Notifications
You must be signed in to change notification settings - Fork 362
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
Test vector transport and its inverse with dIntegrateTransport
#2273
Test vector transport and its inverse with dIntegrateTransport
#2273
Conversation
Great contribution @fabinsch ! Can you also test for matrices ? It should still work the same |
Sure, I will extend it for matrices. One point for me was to check if it actually works for a single vector (
|
perfect! |
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.
Does this test already exist in C++?
we are currently testing this in our cpp unittests for |
Could you add it in C++ instead of Python then? |
49b4c89
to
30a9ad4
Compare
cd66eec
to
6e4be1e
Compare
6e4be1e
to
f2db9df
Compare
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.
(Minor comment.)
// test reverse direction | ||
TangentVector_t v_r = -v; // reverse path | ||
ConfigVector_t qa_r = lg.integrate(qb, v_r); | ||
lg.dIntegrateTransport(qa_r, v_r, tvec_at_qa, tvec_at_qa_r, ARG0); |
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.
Shouldn't this be dIntegrateTransport(qb, v_r, tvec_at_qa, ...)
?
Since we check that qa
is equal to qa_r
below, this line amounts to integrating
Meanwhile this line would fully make sense to me if we start from tvec_at_qa
, and transport it back to tvec_at_qb
and tvec_at_qa_r
below. (But then, tvec_at_qa_r
lies in the tangent space at
@fabinsch I understand the test passed so I must have missed something. If you can double check at some point that would be 👌
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.
Meanwhile I fully agree with the Python variant.
Hi, I propose adding another test for the
dIntegrateTransport
function to check its "inverse" by using the reverse path. I use this function for vector transport along different manifolds.