Skip to content

Make Values.TangentVector == VectorValues #50

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

Merged
merged 3 commits into from
May 12, 2020

Conversation

marcrasi
Copy link
Collaborator

(This is #49, rebased to feature/nlfg_with_manifold. I'm pasting the same description below.)

This sets Values.TangentVector to VectorValues, which allows a few nice improvements to the APIs:

  • To update a Value using a VectorValues, now all you need is vals.move(along: vectorval) instead of needing to loop over all the components and cast them.
  • We can now write a generic JacobianFactor initializer that linearizes any error function, which simplifies the implementation of linearize.

Other API changes:

  • Now you access values using vals[key, as: Pose2.self] instead of vals[key].baseAs(Pose2.self). This is necessary so that Values knows what type you are accessing and therefore knows how to convert its tangent vector to a Vector suitable for inclusion in VectorValues.
  • Now you insert values with vals.insert(key, value) instead of vals.insert(key, AnyDifferentiable(value). (It still gets stored as a type-erased AnyDifferentiable though).

@marcrasi
Copy link
Collaborator Author

Hmm, this build failure is very mysterious because it doesn't happen on Linux and also this PR doesn't seem to touch anything related to it. I'll investigate.

@marcrasi marcrasi force-pushed the marcrasi/values_tangent_vector_rebased branch from 8ec2226 to 7c7c188 Compare May 12, 2020 03:26
@marcrasi
Copy link
Collaborator Author

I figured out the compile error. I have included a workaround, and I also made a proper fix that should appear in toolchains eventually: swiftlang/swift#31723

@ProfFan
Copy link
Collaborator

ProfFan commented May 12, 2020

@marcrasi This PR is great :) Since the feature/nlfg_with_manifold branch is merged into master, should we change the base of this PR to master as well?

@marcrasi marcrasi changed the base branch from feature/nlfg_with_manifold to master May 12, 2020 15:12
@marcrasi
Copy link
Collaborator Author

@marcrasi This PR is great :) Since the feature/nlfg_with_manifold branch is merged into master, should we change the base of this PR to master as well?

Oh yes, done.

@marcrasi marcrasi merged commit eabdbba into master May 12, 2020
@marcrasi marcrasi deleted the marcrasi/values_tangent_vector_rebased branch May 12, 2020 15:21
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