Add kwarg to disable manifold checks in constructor
#540
Conversation
facebook-github-bot
added
the
CLA Signed
theseus/geometry/so3.py
Outdated
| @@ -166,12 +176,14 @@ def _log_map_impl( | |||
| return SO3_base.log(self.tensor, jacobians=jacobians) | |||
|
|
|||
| def _compose_impl(self, so3_2: LieGroup) -> "SO3": | |||
| return SO3(tensor=SO3_base.compose(self.tensor, so3_2.tensor)) | |||
| return SO3( | |||
| tensor=SO3_base.compose(self.tensor, so3_2.tensor), disable_checks=True | |||
There was a problem hiding this comment.
Wondering if we should disable checks for compose() since composition is one of the major sources for the accumulation of numerical errors.
There was a problem hiding this comment.
Changed by strict_checks=False so they are normalized if the check fails.
theseus/geometry/so2.py
Outdated
|
|
||
| def _inverse_impl(self, get_jacobian: bool = False) -> "SO2": | ||
| cosine, sine = self.to_cos_sin() | ||
| return SO2(tensor=torch.stack([cosine, -sine], dim=1), strict=False) | ||
| return SO2(tensor=torch.stack([cosine, -sine], dim=1), strict_checks=False) |
There was a problem hiding this comment.
It is safe for disable_checks=True.
theseus/geometry/so2.py
Outdated
| @@ -220,11 +227,11 @@ def _compose_impl(self, so2_2: LieGroup) -> "SO2": | |||
| cos_2, sin_2 = so2_2.to_cos_sin() | |||
| new_cos = cos_1 * cos_2 - sin_1 * sin_2 | |||
| new_sin = sin_1 * cos_2 + cos_1 * sin_2 | |||
| return SO2(tensor=torch.stack([new_cos, new_sin], dim=1), strict=False) | |||
| return SO2(tensor=torch.stack([new_cos, new_sin], dim=1), strict_checks=False) | |||
There was a problem hiding this comment.
Maybe disable_checks=True?
theseus/geometry/se3.py
Outdated
| @@ -171,10 +181,12 @@ def _log_map_impl( | |||
| return SE3_base.log(self.tensor, jacobians=jacobians) | |||
|
|
|||
| def _compose_impl(self, se3_2: LieGroup) -> "SE3": | |||
| return SE3(tensor=SE3_base.compose(self.tensor, se3_2.tensor)) | |||
| return SE3( | |||
| tensor=SE3_base.compose(self.tensor, se3_2.tensor), disable_checks=True | |||
There was a problem hiding this comment.
Maybe disable_checks=False is better since composition (matrix multiplication) might accumulate numerical errors.
There was a problem hiding this comment.
Changed by strict_checks=False so they are normalized if the check fails.
| return SE3( | ||
| tensor=SE3_base.exp(tangent_vector, jacobians=jacobians), | ||
| disable_checks=True, | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def normalize(tensor: torch.Tensor) -> torch.Tensor: |
There was a problem hiding this comment.
Should normalize() return a group instead of a tensor?
There was a problem hiding this comment.
This was the old API from your initial implementation, but Tensor -> Tensor op makes sense to me.
theseus/geometry/se2.py
Outdated
| @@ -316,7 +323,7 @@ def _compose_impl(self, se2_2: LieGroup) -> "SE2": | |||
| ) | |||
| return SE2( | |||
| tensor=torch.cat([new_translation.tensor, new_rotation.tensor], dim=1), | |||
| strict=False, | |||
| strict_checks=False, | |||
Closes #536