-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add the relative frame task #80
Conversation
Pull Request Test Coverage Report for Build 8458281785Details
💛 - Coveralls |
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.
Maybe I am wrong but line 207 of relative_frame_task.py
look weird to me.
Other than that looks good and I proposed a (tiny) simplification
) | ||
transform_target_to_frame = self.transform_target_to_root.act( | ||
transform_root_to_frame | ||
) |
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.
Should'nt it be transform_root_to_frame.act(self.transform_target_to_root)
?
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.
Woah, at least the variable name (if not more 🙈) is definitely wrong. I will fix that.
task target frame to the root frame. | ||
""" | ||
self.transform_target_to_root = transform_target_to_root.copy() | ||
|
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.
Store the inverse
self.transform_root_to_target = transform_target_to_root.inverse().copy()
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.
This would make sense in internal calculations, but from an API standpoint I think it's better in this direction for consistency with the FrameTask
:
- Frame task has "transform from target to world" (because common practice is to work with
$T_{0b}$ rather than$T_{b0}$ ) - Relative frame task has "transform from target to root"
self.transform_target_to_root | ||
) | ||
error_in_frame: np.ndarray = pin.log(transform_target_to_frame).vector | ||
return error_in_frame |
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.
It becomes:
transform_frame_to_root = configuration.get_transform(
self.frame, self.root
) # rTf
transform_frame_to_target = self.transform_root_to_target.act(
transform_frame_to_root
) # tTf = tTr rTf
error_in_frame: np.ndarray = - pin.log(transform_frame_to_target).vector
# log(fTt) = -log(tTf)
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.
When it is one line of code, you can use the "Add suggestion" button (or <Ctrl+g>) on GitHub.
If it is more than one line of code, either the PR author does it, or you can commit directly to the PR (you are welcome to 😃). Or, in this instance since it was already merged, open a new PR 👌
action_root_to_target @ jacobian_root_in_root | ||
- action_frame_to_target @ jacobian_frame_in_frame | ||
) | ||
return J |
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.
Anycase with the new formulation we can calculate:
with
Recall that
And it simplifies to:
And finaly
And
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.
transform_frame_to_root = configuration.get_transform(
self.frame, self.root
) # rTf
transform_frame_to_target = self.transform_root_to_target.act(
transform_frame_to_root
) # tTf = tTr rTf
action_root_to_frame = transform_frame_to_root.actionInverse
jacobian_frame_in_frame = configuration.get_frame_jacobian(self.frame) # J_f
jacobian_root_in_root = configuration.get_frame_jacobian(self.root) # J_r
J = pin.Jlog6(transform_frame_to_target) @ (
action_root_to_frame @ jacobian_root_in_root
- jacobian_frame_in_frame
) # Je = Jlog (- fXr Jr + J_f)
return J
Thank you @ymontmarin! (And apologies for merging early after our chat, I thought this one was good to go.) I will take care of the fix in #81. Can you open a PR for your alternative calculation proposal? |
This PR adds a task similar to the existing FrameTask, but a bit more general.
It is, on purpose, not a parent/child class of
FrameTask
. We can merge them if applicable in a later PR.Thanks @ymontmarin for helping with the derivation of this task! 👍