search

papagina / RotationContinuity

Coder for "On the Continuity of Rotation Representations"
MIT License
328 stars 54 forks source link

Perhaps bugs when converting axisangle to rotation matrix #7

Open kyang-06 opened 3 years ago

kyang-06 commented 3 years ago

Very impressive and elegant theory to push this field forward!

But I notice there may exist a bug. https://github.com/papagina/RotationContinuity/blob/aee1e3417be2b45f0248c4ca56f34d487a377895/sanity_test/code/tools.py#L172-L180 https://github.com/papagina/RotationContinuity/blob/aee1e3417be2b45f0248c4ca56f34d487a377895/sanity_test/code/tools.py#L141-L147 Code above are the function converting axisangle/rodrigues to rotation matrix. The θ passed to torch.sin and torch.cos is the identity but should be θ/2 instead, referring to Maths - AxisAngle to Quaternion I've hand-written a toy case (rotate y axis by 90 degrees around x axis) and only θ/2 is correct.

I wonder if it is a bug, and if so, does it influence other conversion functions and further influence result of experiments?

papagina commented 3 years ago

Hi, thank you for examining our code! You are right, the function should be θ/2, but since the scaling can be accommodated by the neural network, it wouldn't influence the approximation capability of the network for the representations. To justify it, I also ran the training with the scaled θ and the result remains similar.

meaten commented 2 years ago

Thanks for the great work! I also noticed that compute_rotation_matrix_from_Rodriguez has the same mistakes on halving theta. Could you kindly update tools.py for anyone using your codes?