TRI-ML / dgp

ML Dataset Governance Policy for Autonomous Vehicle Datasets
https://tri-ml.github.io/dgp/
MIT License
94 stars 62 forks source link

schema: add reference coordinate system to Poses #147

Closed DavidTorresOcana closed 11 months ago

DavidTorresOcana commented 1 year ago

Currently, Poses are used to represent SE3 Transformations. A transformation is an affine transformation between two coordinate systems (CS) and, and as such, is bounded to these two (source and destination) coordinate systems.

In other words, a Pose shall indicate the two coordinate system it transforms points in between of.

As, in DGP, each Pose is linked to a datum, a sensor or a piece of information, the only missing CS is the reference (destination) coordinate system.

This PR proposes explicit indication of such Reference Coordinate System on each Pose.

This is in line with ASAM OpenLABEL CS definitions where each piece of information is linked to a CS.

Pending still TODO, but not in the scope of this PR (conversation is encouraged), is how to define and name these CS. ASAM OpenLABEL CS definitions, ISO 8855 and ISO 23150 are a good start.

@rachel-lien @akira-wakatsuki-woven ping

This change is Reviewable

DavidTorresOcana commented 1 year ago

dgp/utils/pose.py line 171 at r2 (raw file):

Previously, rachel-lien wrote…
nit, can you update the docstring parameters section for ` reference_coordinate_system` per style guide? https://github.com/TRI-ML/dgp/blob/master/docs/CONTRIBUTING.md#code-style https://numpydoc.readthedocs.io/en/latest/format.html#parameters

It seems I missed to document these. New commit should address these

DavidTorresOcana commented 1 year ago

dgp/utils/pose.py line 92 at r3 (raw file):

Previously, yuta-tsuzuki-woven wrote…
nit, I think you need to add `Parameters` explanation in the docstring ``` Parameters ---------- new_reference_coordinate_system: str ... ```

oh! I missed this! Thank you for catching it. I also updated the docstring style of that file to match Numpy's

DavidTorresOcana commented 1 year ago

is it ok to merge?

DavidTorresOcana commented 11 months ago

Anything else to do @rachel-lien @yuta-tsuzuki-woven ?