Skip to content

Error in class transforms.Translate #58

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

Closed
hfutcgncas opened this issue Feb 14, 2020 · 5 comments
Closed

Error in class transforms.Translate #58

hfutcgncas opened this issue Feb 14, 2020 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@hfutcgncas
Copy link

hfutcgncas commented Feb 14, 2020

🐛 Error in class pytorch3d.transforms.Translate

I suppose the “translate” part should be in the last column of matrix, not in the last row.

Instructions To Reproduce the Issue:

when I wrote the code like this:

import pytorch3d.transforms as p3d_t
t = p3d_t.Transform3d().translate(1,2,3)
print(t.get_matrix())

I got

tensor([[[1., 0., 0., 0.],
         [0., 1., 0., 0.],
         [0., 0., 1., 0.],
         [1., 2., 3., 1.]]])

while I suppose the correct response should be

tensor([[[1., 0., 0., 1.],
         [0., 1., 0., 2.],
         [0., 0., 1., 3.],
         [0., 0., 0., 1.]]])

I suppose chang the mat[:, :3, 3] to mat[:, 3, :3] in line 388 and 394 of pytorch3d.transforms would fix it.

@hfutcgncas hfutcgncas changed the title Error in class **Translate** Error in class transforms.Translate Feb 14, 2020
@nikhilaravi
Copy link
Contributor

@hfutcgncas this just depends on whether you are using row-major or column-major ordering. As explained in the __init__ function of the Translate class, we assume row-major ordering.

See this guide for a more detailed explanation.

@nikhilaravi nikhilaravi self-assigned this Feb 14, 2020
@hfutcgncas
Copy link
Author

Thank you very much for the explanation.

@hfutcgncas
Copy link
Author

hfutcgncas commented Feb 14, 2020

@nikhilaravi I still have a problem here. In the case of using row-major, I suppose the matrix of RotateAxisAngle(a, axis="X") should be like

m = [ [1,       0,       0,     0],
      [0,  cos(a),  sin(a),     0],
      [0, -sin(a),  cos(a),     0],
      [0,       0,       0,     1]]

but in the code of RotateAxisAngle it is write like

m = [ [1,       0,       0,     0],
      [0,  cos(a), -sin(a),     0],
      [0,  sin(a),  cos(a),     0],
      [0,       0,       0,     1]]

Is there something wrong or I got some mistake about the Coordinate Systems?

@nikhilaravi
Copy link
Contributor

@hfutcgncas you're right, thanks for pointing this out. If we assume row-major ordering, a right hand coordinate system and that positive angles result in counter clockwise rotation then we need to transpose the rotation matrix as you mentioned. I will make the fix ASAP.

@nikhilaravi
Copy link
Contributor

@hfutcgncas This fix has now been landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants