Skip to content
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

[Feature] Transformation #74

Closed
versatran01 opened this issue Feb 18, 2019 · 5 comments
Closed

[Feature] Transformation #74

versatran01 opened this issue Feb 18, 2019 · 5 comments
Labels
enhancement 🚀 Improvement over an existing functionality feature request New feature or request

Comments

@versatran01
Copy link

Hi, I would like to contribute a few functions related to transformation into this repo.
I think just calling things pose is very vague, pose is an abstract term that can have many representations. The current pose is represented using a homogeneous transformation, and it should be named that way (instead of just calling it pose, which can be represented by xyz and quaternion or xyz and euler angles).
Also whenever one mentions such a transformation, it is always better to also mention the two frames (a passive transformation).

The other thing I don't understand is what is eps in all these transformation functions?

@edgarriba
Copy link
Member

hi @versatran01 ! thanks for considering to contribute to the library :D

I totally agree with you about the naming convention. We are totally open to refactor it if that makes sense and the use of the package more clear. Could you list what do you have in mind to implement ? We could start from there to break down and define a proper API for the transforms submodule.

Regarding the epsilon value, this is to prevent NaN values during the backprop. It happened to me in the past that by missing this value, the gradients where quite unstable and prone to exploit. So that's a trick that pytorch core developers recommended to me to put in practice.

@edgarriba edgarriba added enhancement 🚀 Improvement over an existing functionality feature request New feature or request labels Feb 19, 2019
@edgarriba
Copy link
Member

@versatran01 I put this link as reference: https://github.com/ros/geometry/blob/melodic-devel/tf/src/tf/transformations.py

some of the conversions were initially based on this.

@tiancheng-zhi
Copy link

tiancheng-zhi commented Feb 24, 2019

The default epsilon value (1e-6) is too large. Datasets like ScanNet provide camera poses with very high precision (1e-5 ~ 1e-8). The epsilon value should be much smaller than this value.

Also, it would be good if the user can set epsilon by themselves. Currently "DepthWarper" called "relative_transformation" with default eps value (1e-6). The user who is using DepthWarper cannot set the eps value if they want high precision.

@versatran01
Copy link
Author

eps should not be part of the function signature, see #79 for more discussion.

@edgarriba
Copy link
Member

closed in favor of #79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🚀 Improvement over an existing functionality feature request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants