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

transform functions - unintuitive names? #16

Open
gchenfc opened this issue Sep 8, 2020 · 2 comments · Fixed by #72
Open

transform functions - unintuitive names? #16

gchenfc opened this issue Sep 8, 2020 · 2 comments · Fixed by #72
Assignees

Comments

@gchenfc
Copy link
Member

gchenfc commented Sep 8, 2020

I find the naming of the transformTo, transformTwistTo, transformTwistAccelTo, and transformXXXFrom functions to have confusing names and arguments. I can vaguely understand why they were originally named this way, but nonetheless I get confused every time I look at them and, subjectively, I think they could be simplified. I have a possible proposal for what I think might be more intuitive, but feel free to suggest alternatives, or to justify why the existing names make sense (and maybe add it in the docstrings)?

Why they're confusing

Consider transformTo for a joint j connecting links l1 and l2:

  • j.transformTo(l1, q) returns l1_T_l2 aka the pose of l2 in the frame of l1. So transforming from l2 frame to l1 frame. That makes sense and isn't too confusing.

But now consider transformTwistTo:

  • j.transformTwistTo(l1, q, qdot, other_twist) does not return l1_V_l2 aka the twist of l1 relative to the twist of l2 - that doesn't really make any sense. It also does not return $^{l1}\mathcal{V}_{l2}$ aka the twist of l1 in the frame of l2, which might make sense. Instead, it returns the twist of link l2 (expressed in frame l2, I believe). Wouldn't something like twistOf or calculateTwistOf or something make more sense?

To clarify, it seems like j.transformTwistTo(l1, q, qdot, twist_l2) could mean many things if we assume the same wording as j.transformTo (I will use the notation x_V_y to denote the twist of y expressed in the frame of x):

  • l1_V_l2 = [Ad (l1_T_l2)] * l2_V_l2 aka the twist of l2 in the frame of l1. In other words, transform other_twist to frame l1
  • w_V_l1 - w_V_l2 aka the "relative twist" from l2 to l1 - does this even make sense? I don't think so
  • l2_V_l1 - l2_V_l2 It's not really clear what frame the two twists should be
  • w_V_l1 where twist_l2 is given in world frame, ie w_V_l2
  • l1_V_l1 aka the twist of l1 in the frame of l1. This is what the function actually does, but this doesn't match the definition of transformTo at all.

I think this confusion stems from the fact that this function isn't really transforming anything at all. It's just calculating the twist of the next link. I think transformTo makes sense to me because "transform" is another word for "pose" but not because it's transforming something into l1's frame. In other words, I interpret transformTo as using the word "transform" as a noun rather than a verb, but this doesn't make sense for transformTwistTo.

What's the point of transformXXXFrom ?

Why do we even need the transformXXXFrom versions? Based on a cursory "find-in-files" for transformFrom, it looks like it's never really used. It's used once in Robot.cpp, but I'd say this is totally frivolous since (a) both link1 and link2 are declared and (b) the variable it gets assigned to is actually never even used so that entire line could be deleted... The remaining 21 times it's used are in unit tests just to test that transformFrom was implemented correctly for the various joints. Doesn't that seem a little bit absurd?

pMcCom and cMpCom

As another minor note, why do the pMcCom(q) and cMpCom(q) functions use M rather than T? I thought that M represented the rest transform?

Proposal

I would like to lightly propose to switch to the following functions:

  • relativePoseOf(l1, q) = l2_T_l1
  • poseOf(l1, q, w_T_l2) = w_T_l1 - of course w needn't be the world frame, and passing the identity as w_T_l2 would reduce to the previous bullet point
  • twistOf(l1, q, qdot, l2_V_l2) = l1_V_l1 - frame l1 instead of world frame because that's the default in Lynch & Park
  • twistAccelOf(l1, q, qdot, qddot, l1_V_l1, l2_A_l2) = l1_A_l1)

If desired, the following functions can also be used if you really like the "to/from" but want to be a little more consistent:

  • transformTo(l1, q, w_T_l2) = w_T_l1 = poseOf(l1, q, w_T_12)
  • transformTwistTo(l1, q, qdot, l2_V_l2) = l1_V_l2 = [Ad (l1_T_l2)] * l2_V_l2
  • transformTwistAccelTo(l1, q, qdot, qddot, l2_A_l2) = l1_A_l2 = [Ad (l1_T_l2)] * l2_A_l2

Again, this is just a possible alternative, but feel free to suggest alternatives.

Summary

Personally, I find the wording of transformXXXTo functions confusing. Subjectively, I feel the names don't accurately describe what they do and I always get mixed up. Therefore, I have explained what I personally find confusing about them and given a proposed alternative. Perhaps someone could either enlighten me as to the reasoning behind their naming (and update the documentation) or suggest/approve alternative namings?

@varunagrawal
Copy link
Collaborator

I believe this was at least partially fixed by #72?

@varunagrawal varunagrawal linked a pull request Mar 22, 2021 that will close this issue
@dellaert
Copy link
Member

Re-reading this, and I agree with most of the above! I wish I would have had more time to engage with you guys back then - I was doing other things :-/

Let's discuss in GTDynamics meeting :-)

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 a pull request may close this issue.

6 participants