Fix convexhull boundary computation #244
Conversation
argoverse/utils/transform.py
Outdated
| @@ -48,6 +55,13 @@ def quat_argo2scipy(q: np.ndarray) -> np.ndarray: | |||
| return q_scipy | |||
|
|
|||
|
|
|||
| def quat_scipy2argo(q: np.ndarray) -> np.ndarray: | |||
There was a problem hiding this comment.
This is on a bit of a meta note, but we really need to clean up transform.py (the file as a whole).
johnwlambert
changed the title
johnwlambert
changed the title
|
@tagarwal-argoai would you mind taking a look at this, also? |
| return Polygon(poly) | ||
| # `simplices` contains indices of points forming the simplical facets of the convex hull. | ||
| poly_pts = hull.points[np.unique(hull.simplices)] | ||
| return Polygon(poly_pts) |
There was a problem hiding this comment.
@janesjanes and @benjaminrwilson would you mind taking a brief look at this?
I believe the previous code was actually mixing the x,y,z coordinates of 3 vertices, which I don't think was the intended behavior.
If a simplicial facet is defined by vertex 0 = (x0,y0,z0), vertex 1=(x1,y1,z1), vertex 2=(x2,y2,z2), then I think we were forming (x0,x1,x2) as a point of the polygon, which wouldn't be correct.
There was a problem hiding this comment.
Thanks for looking into this, @johnwlambert. Before I dive in, are we using this function anywhere?
There was a problem hiding this comment.
Thanks Ben. So this is used in a tracking competition demo notebook: https://github.com/argoai/argoverse-api/blob/master/demo_usage/competition_tracking_tutorial.ipynb
| return (pt1[0] - pt0[0]) / dis_0_to_1, (pt1[1] - pt0[1]) / dis_0_to_1 | ||
|
|
||
| def poly_to_label(poly: Polygon, category: str = "VEHICLE", track_id: str = "") -> ObjectLabelRecord: | ||
| """Convert a Shapely Polygon to a 3d cuboid by estimating the minimum-bounding rectangle. |
There was a problem hiding this comment.
I tried to simplify this logic a bit here, and make things a bit clearer.
There was a problem hiding this comment.
Does this get called outside of this file?
There was a problem hiding this comment.
| "x": label.quaternion[0], | ||
| "y": label.quaternion[1], | ||
| "z": label.quaternion[2], | ||
| "w": label.quaternion[3], |
There was a problem hiding this comment.
@janesjanes since we express quaternions as qw, qx, qy, qz, I'm not sure this is correct here.
Since the quaternion package uses qw, qx, qy, qz, but we are unpacking this as qx, qy, qz, qw.
| # z = poly.exterior.xy[2] | ||
|
|
||
| d1 = dist((x[0], y[0]), (x[1], y[1])) | ||
| d2 = dist((x[1], y[1]), (x[2], y[2])) |
There was a problem hiding this comment.
i thought this indexing was a bit hard to follow, so i tried to simplify it a bit
| @@ -220,7 +206,7 @@ def poly_to_label(poly: Polygon, category: str = "VEHICLE", track_id: str = "") | |||
|
|
|||
|
|
|||
| def get_objects(clustering: DBSCAN, pts: np.ndarray, category: str = "VEHICLE") -> List[Tuple[np.ndarray, uuid.UUID]]: | |||
|
|
|||
| """ """ | |||
There was a problem hiding this comment.
Could we provide a small docstring here?
… remove-quaternion-dependency
johnwlambert
changed the title
johnwlambert
changed the title
Switch to fully using
scipy(8.3k stars) instead ofquaternion(413 stars) for rotation matrix -> quaternion conversions.Note: the
quaternionlibrary uses scalar-first orderqw, qx, qy, qz(see this issue), whereas scipy uses scalar-last order, so we re-order the coefficients insideyaw_to_quaternion3d().Resolves the issue mentioned in #239.