-
Notifications
You must be signed in to change notification settings - Fork 935
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
Update doxygen comments for distance() and interpolate() #2528
Conversation
60c7893
to
6ae0dcc
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.
Thank you for doing this! I read through these and they look good. I'll merge this once it passes CI if no one else requests changes.
Codecov Report
@@ Coverage Diff @@
## master #2528 +/- ##
==========================================
- Coverage 60.23% 60.22% -0.00%
==========================================
Files 351 351
Lines 26470 26470
==========================================
- Hits 15942 15940 -2
- Misses 10528 10530 +2
Continue to review full report at Codecov.
|
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.
thanks especially for the corner-case info about active joints!
`distance()` just sums the joint difference, for all joints. It's not an L2 norm or SSE or anything like that.
Just because of the way you formulate this: summing up element-wise differences is the standard [L1 norm](https://en.wikipedia.org/wiki/Taxicab_geometry) and I'm missing that term a bit in your patch. There was even a proposal to add weighting to the components, but it was not merged as far as I know.
|
I'm porting this to MoveIt2 now and will mention L1 norm 👍 |
Update documentation of
interpolate()
anddistance()
to clarify a few things:t
parameter of interpolate isn't a time, it's a fractiondistance()
just sums the joint difference, for all joints. It's not an L2 norm or SSE or anything like that. Verify here.Fixes #2527