-
Notifications
You must be signed in to change notification settings - Fork 78
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
3D Object Detection Evaluation #31
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good overall, Ben. Left some comments.
Each sweep is processed independently, computing assignment between detections and ground truth annotations. | ||
|
||
Args: | ||
dts: (N,15) Table of detections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could complement the args with, e.g.,: Table with N detections and 15 attributes? (e.g., x, y, z ...).
# # Check that there are 2 regular vehicles. | ||
# assert gts["category"].value_counts()["REGULAR_VEHICLE"] == 2 | ||
|
||
# # Check that there are no other labels. | ||
# assert gts["category"].value_counts().sum() == 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't we use these?
breakpoint() | ||
# read_city_SE3_ego() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove breakpoint and the commented function?
Valid cuboids meet _two_ conditions: | ||
1. The cuboid's centroid (x,y,z) must lie within the maximum range in the detection configuration. | ||
2. The cuboid must have at _least_ one point in its interior. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring might be a bit confusing as we are not using the number of points condition in this function. Or should we add is_valid_num_points: NDArrayBool = gts[:, -1] > 0
here too?
dts: (N,15) Table of detections. | ||
gts: (M,15) Table of ground truth annotations. | ||
cfg: Detection configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add meaning for 15 here (e.g., where 15 is the number of attributes (e.g., x, y, z).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks, Ben!
PR Summary
Testing
In order to ensure this PR works as intended, it is:
Compliance with Standards
As the author, I certify that this PR conforms to the following standards: