feat: Add ProbabilisticKeyPoint3D #171
Conversation
52dc7a8 to
b7958ec
Compare
dgp/utils/structures/key_point_3d.py
Outdated
| assert np.allclose(mat, mat.T, rtol=rtol, atol=atol), f"{mat} is not symmetric!" | ||
|
|
||
| @staticmethod | ||
| def _assert_positive_semi_definite(mat: np.ndarray) -> None: |
There was a problem hiding this comment.
I think this is actually checking for positive definite. If we want to allow the zero matrix (the default value) we have to allow 0 eignvalues.
Also, is this enough to conclude the diagonal elements are always non negative?
There was a problem hiding this comment.
Yes, you are right, this is actually positive definite. If for instance the var_z is zero it means that in the Z direction there is no variance which is not realistic, I will revise the function name.
Also yes, the diagonal elements are guaranteed to be non-negative.
There was a problem hiding this comment.
I see, but the default value in the proto will be 0, so I think we should either support the zero matrix (which has 0 eigenvalues) or we need to set a default value in the proto (via default keyword) that is large.
There was a problem hiding this comment.
I set the matrix validation check to False by default. Also it looks like we could not set default value in proto3. In this case I think it would make more sense to ensure the value on the caller side.
f520398 to
d9920df
Compare
efd5679 to
21b9d0a
Compare
d829004 to
dae233e
Compare
| @@ -54,7 +55,8 @@ | |||
| "key_point_3d": KeyPoint3DAnnotationList, | |||
| "key_line_2d": KeyLine2DAnnotationList, | |||
| "key_line_3d": KeyLine3DAnnotationList, | |||
| "depth": DenseDepthAnnotation | |||
| "probabilistic_key_line_3d": ProbabilisticKeyLine3DAnnotationList, | |||
There was a problem hiding this comment.
I believe only if you want support for this type in the legacy dataloaders? I'm not sure if this is sufficient.
| @staticmethod | ||
| def _get_mat(data: np.ndarray) -> np.ndarray: | ||
| assert data.shape == (6, ), f"data.shape={data.shape} != (6,)!" | ||
| var_x = data[0] |
There was a problem hiding this comment.
nit: could this be unpacked like a tuple?
dgp/utils/math.py
Outdated
| [cov_xy, var_y, cov_yz], | ||
| [cov_xz, cov_yz, var_z], | ||
| ], | ||
| dtype=np.float32, |
There was a problem hiding this comment.
nit: maybe should be a double to avoid precision issues that could lead to an invalid covariance matrix?
dgp/utils/render_3d_to_2d.py
Outdated
| @@ -1,4 +1,6 @@ | |||
| # Copyright 2023 Toyota Motor Corporation. All rights reserved. | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
why do we need this? we don't support python2
|
|
||
| @property | ||
| def instance_ids(self) -> np.ndarray: | ||
| return np.array([line.instance_id for line in self._linelist], dtype=np.int64) |
There was a problem hiding this comment.
I think this dtype can cause problems. instance_id is optional and when None gets set to a string. This is a known issue in BoundingBox3DAnnotationList where printing throws an error
dgp/utils/structures/key_line_3d.py
Outdated
| @property | ||
| def hexdigest(self): | ||
| cov_bytes = np.asarray([point.cov3.arr6 for point in self.points]).tobytes() | ||
| return hashlib.md5(self.xyz.tobytes() + cov_bytes + bytes(self._class_id)).hexdigest() |
There was a problem hiding this comment.
nit: do we need to hash the attributes?
dgp/utils/structures/key_point_3d.py
Outdated
|
|
||
| @property | ||
| def hexdigest(self): | ||
| return hashlib.md5(self.xyz.tobytes() + self.cov3.arr6.tobytes() + bytes(self._class_id)).hexdigest() |
dgp/utils/visualization_utils.py
Outdated
| @@ -593,17 +616,47 @@ def render_paths(self, paths, extrinsics=Pose(), colors=(GREEN, ), line_thicknes | |||
|
|
|||
| combined_transform = self._bev_rotation * extrinsics | |||
|
|
|||
| for path, color in zip(paths, colors): | |||
| # Compute the yaw angle from quaternion. | |||
There was a problem hiding this comment.
if it is a pose object you can do pose.quat.yaw_pitch_roll
| angle=yaw_angle, | ||
| startAngle=0.0, | ||
| endAngle=360.0, | ||
| color=(0, 255, 0), |
There was a problem hiding this comment.
nit: would be nice to be able to set color (different color for left/right etc)
dgp/utils/visualization_utils.py
Outdated
| startAngle=0.0, | ||
| endAngle=360.0, | ||
| color=(0, 255, 0), | ||
| thickness=1, |
There was a problem hiding this comment.
maybe use line_thickness?
tests/utils/test_camera.py
Outdated
| @@ -1,6 +1,5 @@ | |||
| import numpy as np | |||
| import pytest | |||
| from numpy.lib.ufunclike import fix | |||
9eb2391 to
43126d4
Compare
a818ba4 to
9d131b8
Compare
9d131b8 to
1d1f163
Compare
No description provided.