-
-
Notifications
You must be signed in to change notification settings - Fork 939
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
Inconsistent conventions in function names #2781
Comments
what is the confusion here ? even though you can use one to solve the other, in principle the functions in principle are unrelated. The first, applies a generic transformation to coordinates system of a set points given the mapping transformation points_in_robot = transform_points(robot_from_world, points_in_world) the second has a specific purpose to project a set of 3d points to the z1 image plane given the calibration parameters of an affine model. The last is an old function that's is planned to be replaced by the new cameras api (based on sophus conventions): https://kornia.readthedocs.io/en/latest/geometry.camera.html |
Well, it would be really simple and great, if we use same idea - points first, or points last, across all the library. |
This inconsistency makes it confusing to use.
For me this is by far the most common usecase of these functions. Am I supposed to be creating I mean I can just write these functions myself instead, but I'd like to be able to use standardized stuff. |
@Parskatt thanks for the feedback. The The new apis are coming from Sophus / sophus-rs and a bit of Pangolin that have years of iterations for production in known companies and we use internally in our group for camera calibration and slam stuff. Possibly kornia-rs will adopt some of this too since the idea is to converge more and more to the Sophus api.
That's the context with all the why's for this apis -- said that, i'm totally happy that we define together a standard apif for the calibration / geometry api that makes sense for kornia context and whom we useit everyday. If you like we can open a design docs and discuss there the apis. Keep in mind backward compatibilities and such based on our new releases schedule. /cc @cjpurackal since he's involved too with the new sensors and lie api |
@edgarriba exactly, we have inconsistent inspirations, and we should unify them. Users don't care about the history |
that was an arbitrary choice i guess when that function was created. Depending on the application you would like in one or the other side, I'd be in favor in deprecating that and just teaching our users to use as follows: these all should be equivalent (rigid body transform and points conventions are explained here) t1_from_t2 = compose_transform(t1_from_t0, t0_from_t2)
t1_from_t2 = compose_transform(t0_from_t1.inverse(), t0_from_t2)
t1_from_t2 = compose_transform(t1_from_t0, t2_from_t0.inverse())
t1_from_t2 = compose_transform(t0_from_t1.inverse(), t2_from_t0.inverse())
t1_from_t2 = compose_transform(t2_from_t0, t1_from_t0.inverse()).inverse() where |
Describe the bug
Here the points go second
https://kornia.readthedocs.io/en/latest/geometry.linalg.html#kornia.geometry.linalg.transform_points
Here first
https://kornia.readthedocs.io/en/latest/geometry.camera.perspective.html
Reproduction steps
Expected behavior
Not be confusing.
Environment
Additional context
No response
The text was updated successfully, but these errors were encountered: