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

addPriorDouble #730

Merged
merged 1 commit into from
Apr 5, 2021
Merged

addPriorDouble #730

merged 1 commit into from
Apr 5, 2021

Conversation

dellaert
Copy link
Member

@dellaert dellaert commented Apr 4, 2021

Added double as template argument

Copy link
Member

@gchenfc gchenfc left a comment

Choose a reason for hiding this comment

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

Could we also add the fixed-vector versions:
Vector1, Vector2, Vector3, Vector4, Vector5, Vector6 ?

Might be a little bit excessive but otherwise causes annoying errors:

Attempting to retrieve value with key "24209042725404672", type stored in Values is
gtsam::GenericValue<Eigen::Matrix<double, 6, 1, 0, 6, 1> > but requested type was
Eigen::Matrix<double, -1, 1, 0, -1, 1>

that are quite annoying to work around from the Python side.
*edit, I guess strictly speaking Vector2 and Vector3 aren't necessary since one could use Point2 and Point3, and Vector1 should be quite rare... But Vector6 is quite common in GTDynamics

@dellaert
Copy link
Member Author

dellaert commented Apr 4, 2021

@gchenfc feel free to do a different PR. But the possible confusion between static and dynamic vector types makes that a less straightforward PR. In any case, orthogonal to this one, which solves a simple problem in GTDynamics.

@dellaert
Copy link
Member Author

dellaert commented Apr 4, 2021

@gchenfc please approve this so I can merge :-) CI seems OK, only the one time-out...

@gchenfc gchenfc self-requested a review April 4, 2021 23:52
@dellaert dellaert merged commit 37ea955 into develop Apr 5, 2021
@dellaert dellaert deleted the feature/addPriorDouble branch April 5, 2021 03:11
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