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

Replace AdjointMapJacobianQ with gtsam::Adjoint w/ Jacobians #292

Merged
merged 28 commits into from
Jul 13, 2022

Conversation

gchenfc
Copy link
Member

@gchenfc gchenfc commented Oct 16, 2021

After borglab/gtsam#885 merges, then this will use, e.g.

Pose3 T = joint->parentTchild(q, T_H_q);
Vector6 twist_p = T.Adjoint(twist_c, H_T, H_twist_c);
H_q = H_T * T_H_q;

instead of:

Vector6 twist_p = T.Adjoint() * twist_c;
H_q = AdjointMapJacobianQ(joint, q) * twist_c;

This is just much more sane and generalizeable and uses more GTSAM code instead of custom code.

@gchenfc gchenfc changed the base branch from master to refactor/kill_JointTyped October 16, 2021 08:10
@gchenfc gchenfc force-pushed the refactor/kill_JointTyped branch 2 times, most recently from 7f66e4a to 3d0930a Compare October 27, 2021 18:55
Base automatically changed from refactor/kill_JointTyped to master October 27, 2021 21:33
@varunagrawal
Copy link
Collaborator

@gchenfc I am undoing some duplicate changes from #290 since those changes are moot given the current state of master. I'm waiting for folks on Office Hours so going to use this time to land this. :)


// Calculations
gtsam::Pose3 T_21 = joint_->childTparent(q, H_q ? &T_21_H_q : nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gchenfc you may want to re-add this in the wrenchEquivalence constraint.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm adding it. I figured it out.

Gaussian::shared_ptr cost_model = Gaussian::Covariance(gtsam::I_6x6);
gtsam::Key wTp_key = internal::PoseKey(1), wTc_key = internal::PoseKey(2),
q_key = internal::JointAngleKey(1);
size_t k = 777;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh we'll have to re-add these tests again. @gchenfc man, we need to compartmentalize PRs better :/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually these test changes are in #290.

Copy link
Collaborator

@varunagrawal varunagrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made updates to get this working so we can merge whenever!

@varunagrawal varunagrawal merged commit 57575de into master Jul 13, 2022
@varunagrawal varunagrawal deleted the refactor/AdjointMapJacobians branch July 13, 2022 04:13
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 this pull request may close these issues.

2 participants