add adjoint, hat and vee for SE3 #68
Conversation
facebook-github-bot
added
the
CLA Signed
theseus/geometry/se3.py
Outdated
| return ret | ||
|
|
||
| def update_from_x_y_z_quaternion(self, x_y_z_quaternion: torch.Tensor): | ||
| if x_y_z_quaternion.ndim != 2 and x_y_z_quaternion.shape[1] != 7: |
There was a problem hiding this comment.
We want to handle the case where the batch dimension is not passed and therefore batch size is assumed to be 1 (possibly also check in other places if we account for this).
There was a problem hiding this comment.
OK. I will add this feature.
There was a problem hiding this comment.
TBH, I'm not sure if we support this in 2D functions. Should we open an issue to remember to check for this?
There was a problem hiding this comment.
There is SO2.update_from_angle but SE2 seems to have no similar functions.
There was a problem hiding this comment.
TBH, I'm not sure if we support this in 2D functions. Should we open an issue to remember to check for this?
Okay with either of these but we should be consistent:
a. We clarify in docs that batch dimension is always present and users need to add a stub dim for a batch size of one.
b. We let users define things without batch and append the extra dim internally everywhere.
There was a problem hiding this comment.
Actually, for some reason I was thinking of something else. In Lie group classes constructors we do automatically append batch dimensions if not present, and @fantaosha also added this here in line 35. I guess the question is if it makes sense to move the unsqueeze inside the updated_from_x_y_z_quaternion function, which I think it does.
There was a problem hiding this comment.
Sounds good, option b then. Yes, we should move the unsqueeze. In general, every class and function should support automatic appending of batch dim then.
There was a problem hiding this comment.
Let's move the unsqueeze for adding a batch dimension (if needed) inside updated_from_x_y_z_quaternion before merging.
There was a problem hiding this comment.
@fantaosha @luisenp was this addressed before merging? If not, maybe add to an active PR.
theseus/geometry/se3.py
Outdated
| return ret | ||
|
|
||
| def update_from_x_y_z_quaternion(self, x_y_z_quaternion: torch.Tensor): | ||
| if x_y_z_quaternion.ndim != 2 and x_y_z_quaternion.shape[1] != 7: |
There was a problem hiding this comment.
TBH, I'm not sure if we support this in 2D functions. Should we open an issue to remember to check for this?
* add SE3 defintions * add initialization method * modify test_so3.check_SO3_to_quaternion to handle cases near pi * add SE3.hat and SE.vee * add SE3.adjoint * add SE3.adjoint, vee and hat * backup before switching to taoshaf.add_SE3 * add support for single x_y_z_quaternion * modify SO3 * Remove repeated if in SE3 construction
Motivation and Context
How Has This Been Tested
Types of changes
Checklist