Refactor SO3 and SE3 to be consistent with functorch #266
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fantaosha
commented
Aug 18, 2022
fantaosha
commented
Aug 18, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 26, 2022
mhmukadam
reviewed
Aug 31, 2022
mhmukadam
reviewed
Aug 31, 2022
mhmukadam
reviewed
Aug 31, 2022
luisenp
reviewed
Aug 31, 2022
mhmukadam
approved these changes
Sep 7, 2022
luisenp
approved these changes
Sep 7, 2022
| tensor = self._check_tensor(tensor, strict) | ||
| else: | ||
| warnings.warn( | ||
| f"functorch is enabled and tensor is not checked " |
There was a problem hiding this comment.
We should change this message as well, since this now has nothing to do with functorch. Maybe "Lie group consistency checks are disabled."? Feel free to change in #268 if that's easier.
| ) | ||
| else: | ||
| warnings.warn( | ||
| "functorch is enabled and the skew-symmetry of hat matrices is " |
There was a problem hiding this comment.
Need to change this message as well. How about "Lie group checks are disabled, so the skew-symmetry... etc."?
| _check &= torch.allclose(matrix[:, 0, 1], -matrix[:, 1, 0]) | ||
| else: | ||
| warnings.warn( | ||
| "functorch is enabled and the skew-symmetry of hat matrices is " |
suddhu
pushed a commit
to suddhu/theseus
that referenced
this pull request
Jan 21, 2023
…h#266) * fixed some bugs in SO3.log_map * refactor SO3 to be consistent with functorch * fixed a bug in SO3._project_impl * add more tests for SO3 * SE3 refactored to be consistent with functorch * simplify SO3 and SE3 for functorch * refactor so2 to be consistent with functorch * torch.zeros() -> tensor.new_zeros() * simplify the code using new_zeros * refactor se2 * refactor the projection map for SE3 * fixed a bug in SO2._rotate_from_cos_sin * fixed a bug for functorch * refactor SO3.log_map_impl * refactor SO3 and remove functorch context for log_map_impl() and to_quaternion() * refactor SE3._log_map_impl * SO3 refactored * functorhc refactored * add more warning info for functorch * fixed a bug in warnings message about tensor check for functorch * rename functorch context * rename lie_group_tensor to lie_group * some changes are made * rename lie_group_tensor_check to lie_group_check * fixed the logic bug
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
How Has This Been Tested
Types of changes
Checklist