-
Notifications
You must be signed in to change notification settings - Fork 129
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 ManifoldGaussian class for messages in belief propagation #121
Conversation
I assume that all Manifold instances passed in the mean Sequence have the same batch size, dtype and device. |
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.
Looks good so far! On to unit tests. Any other features you are considering adding still?
I don't think so. I was thinking of adding the local and retract functions but generalised to gaussians on manifold variables, but they would be better placed in the Manifold class. |
Okay probably add it in this PR. Manifold already has a local and retract, how will this Gaussian version get incorporated? |
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.
Looks ready once we decide whether to move the local/retract inside the class.
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.
LGTM! Is the plan to keep local/retract outside of the class?
Should be ready to merge now. |
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.
Looks good! Left some comments/questions. I didn't check the math closely.
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.
Look good to merge, nice work!
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.
LGTM!
* Change unit tests to avoid making mypy a main requirement. (#168) * Change unit tests to avoid making mypy a main requirement. * Added back dev requirements to CI. * Wording change. * Update readme and contrib (#169) * Change unit tests to avoid making mypy a main requirement. * Added back dev requirements to CI. * Removed thanks file. * Add missing license header in test_urdf_model * Add ManifoldGaussian class for messages in belief propagation (#121) * Gaussian class to wrap Manifold class and lam matrix for inverse covariance * reformatted * restored original manifold file * initial attempt at marginal class , need to handle batch dim * added standard fns dtype, to, copy, update * single to call in init * renamed ManifoldGaussian * setting precision in init with checks * update function requires mean and precision * fixed naming in init * manifold gaussian tests * retract and local gaussian fns * check precision is a symmetric matrix * moved retract and local gaussian to manifold_gaussian to avoid circular imports * added ManifoldGaussian to inits * minor edits * fixed dtype error in se3 that appeared in unit tests * add checks for local_gaussian * tests for local and retract gaussian * import from th. * added local_gaussian retract_gaussian to init, minor fix * minor fix on value error * fixed copy bug and added comments * random precision matrix in unit tests * fix for random precision * init precision with identity * fixed typo * More efficient implementation of forward kinematics (#175) * Bump DRM version and adapt API * Add small tolerences to tests to account for numerical errors * Changing setup virtualenv command. (#178) * Updated SDF object in collision cost functions whenever an aux var is updated (#177) * Updated SDF object in collision cost functions whenever an aux var is updated. * Changed gather_from_rows_cols to support matrix broadcasting. Co-authored-by: Mustafa Mukadam <[email protected]> Co-authored-by: Joe Ortiz <[email protected]> Co-authored-by: Austin Wang <[email protected]>
* Gaussian class to wrap Manifold class and lam matrix for inverse covariance * reformatted * restored original manifold file * initial attempt at marginal class , need to handle batch dim * added standard fns dtype, to, copy, update * single to call in init * renamed ManifoldGaussian * setting precision in init with checks * update function requires mean and precision * fixed naming in init * manifold gaussian tests * retract and local gaussian fns * check precision is a symmetric matrix * moved retract and local gaussian to manifold_gaussian to avoid circular imports * added ManifoldGaussian to inits * minor edits * fixed dtype error in se3 that appeared in unit tests * add checks for local_gaussian * tests for local and retract gaussian * import from th. * added local_gaussian retract_gaussian to init, minor fix * minor fix on value error * fixed copy bug and added comments * random precision matrix in unit tests * fix for random precision * init precision with identity * fixed typo
…okresearch#121) * Gaussian class to wrap Manifold class and lam matrix for inverse covariance * reformatted * restored original manifold file * initial attempt at marginal class , need to handle batch dim * added standard fns dtype, to, copy, update * single to call in init * renamed ManifoldGaussian * setting precision in init with checks * update function requires mean and precision * fixed naming in init * manifold gaussian tests * retract and local gaussian fns * check precision is a symmetric matrix * moved retract and local gaussian to manifold_gaussian to avoid circular imports * added ManifoldGaussian to inits * minor edits * fixed dtype error in se3 that appeared in unit tests * add checks for local_gaussian * tests for local and retract gaussian * import from th. * added local_gaussian retract_gaussian to init, minor fix * minor fix on value error * fixed copy bug and added comments * random precision matrix in unit tests * fix for random precision * init precision with identity * fixed typo
Motivation and Context
It would be useful to have an optional covariance / precision matrix as part of the Manifold class as Gaussian Belief Propagation involves sending Gaussian distributions over the Manifold variables. Currently I'm using a wrapper Gaussian class but it could be more widely useful to have covariance / precision matrix as an attribute of the Manifold class?
How Has This Been Tested
Types of changes
Checklist