Skip to content
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

Merged
merged 73 commits into from
Apr 11, 2022
Merged

Conversation

benjaminrwilson
Copy link
Collaborator

@benjaminrwilson benjaminrwilson commented Mar 28, 2022

PR Summary

  • Adds 3D Object Detection Evaluation API

Testing

In order to ensure this PR works as intended, it is:

  • unit tested.
  • other or not applicable (additional detail/rationale required)

Compliance with Standards

As the author, I certify that this PR conforms to the following standards:

  • Code changes conform to PEP8 and docstrings conform to the Google Python style guide.
  • A well-written summary explains what was done and why it was done.
  • The PR is adequately tested and the testing details and links to external results are included.

@benjaminrwilson benjaminrwilson changed the title 3D Detection Evaluation 3D Object Detection Evaluation Mar 28, 2022
@benjaminrwilson benjaminrwilson marked this pull request as ready for review March 31, 2022 18:07
Copy link
Contributor

@jhonykaesemodel jhonykaesemodel left a 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.
Copy link
Contributor

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 ...).

src/av2/evaluation/detection/eval.py Outdated Show resolved Hide resolved
src/av2/evaluation/detection/eval.py Outdated Show resolved Hide resolved
tests/evaluation/detection/test_eval.py Outdated Show resolved Hide resolved
Comment on lines +178 to +182
# # 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
Copy link
Contributor

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?

Comment on lines 323 to 324
breakpoint()
# read_city_SE3_ego()
Copy link
Contributor

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?

src/av2/evaluation/detection/utils.py Outdated Show resolved Hide resolved
Comment on lines 358 to 361
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.

Copy link
Contributor

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?

src/av2/evaluation/detection/utils.py Outdated Show resolved Hide resolved
Comment on lines +171 to +173
dts: (N,15) Table of detections.
gts: (M,15) Table of ground truth annotations.
cfg: Detection configuration.
Copy link
Contributor

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).

Copy link
Contributor

@jhonykaesemodel jhonykaesemodel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks, Ben!

@benjaminrwilson benjaminrwilson merged commit 5cbeb6d into argoverse:main Apr 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants