fix(perception_fp): change method contain to intersect#333
fix(perception_fp): change method contain to intersect#333MasatoSaeki wants to merge 1 commit intodevelopfrom
Conversation
Signed-off-by: MasatoSaeki <masato.saeki@tier4.jp>
There was a problem hiding this comment.
Pull request overview
This PR updates the Perception FP (false positive) evaluation logic for bbox data by switching the non-detection-area XY check from point containment to polygon intersection, and extends the sample scenario with an additional bbox-only criterion.
Changes:
- Update bbox vs non-detection-area XY evaluation from
contains(...)on corners tointersects(...)on a bbox polygon. - Add a new
Criterionentry to the sampleperception_fpscenario for bbox evaluation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py |
Changes bbox-in-area evaluation predicate from corner containment to polygon intersection. |
sample/perception_fp/scenario.yaml |
Adds an additional bbox-only criterion sample configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| prep_geom, corners[:, 0], corners[:, 1] | ||
| ) # or contains(non_detection_area, Polygon(corners[:, 0:2])) | ||
| ) | ||
| is_in_xy = np.any(prep_geom.intersects(Polygon(corners[:, 0:2]))) |
There was a problem hiding this comment.
prep_geom.intersects(...) returns true even when the bbox only touches the non-detection area boundary. This conflicts with the documented behavior for perception_fp (boundary contact should be treated as non-contact) and with the local comment # boundary is not included. Consider using an interior-intersection check (e.g., intersects AND NOT touches, or another predicate that excludes boundary-only contact) so boundary cases don't become false positives.
| is_in_xy = np.any(prep_geom.intersects(Polygon(corners[:, 0:2]))) | |
| polygon_xy = Polygon(corners[:, 0:2]) | |
| # boundary is not included: require interior intersection (not just touching) | |
| is_in_xy = prep_geom.intersects(polygon_xy) and not non_detection_area.geom.touches( | |
| polygon_xy | |
| ) |
| is_in_xy = np.any(prep_geom.intersects(Polygon(corners[:, 0:2]))) | ||
| if is_in_z and is_in_xy: |
There was a problem hiding this comment.
This change alters the bbox-in-polygon logic (from checking corner containment to polygon intersection). Please add unit tests that cover: (1) a bbox that overlaps the non_detection_area even though all its corners are outside, and (2) a bbox that only touches the boundary (should be treated as non-contact per the use-case docs). Current tests only cover the fully-inside case, so they won't catch regressions here.
| prep_geom, corners[:, 0], corners[:, 1] | ||
| ) # or contains(non_detection_area, Polygon(corners[:, 0:2])) | ||
| ) | ||
| is_in_xy = np.any(prep_geom.intersects(Polygon(corners[:, 0:2]))) |
There was a problem hiding this comment.
np.any(...) is redundant here because prep_geom.intersects(...) already returns a single boolean. Removing the np.any wrapper will make the intent clearer and avoid implying vectorized behavior.
| is_in_xy = np.any(prep_geom.intersects(Polygon(corners[:, 0:2]))) | |
| is_in_xy = prep_geom.intersects(Polygon(corners[:, 0:2])) |
|
Types of PR
Description
This PR fixes the
How to review this PR
Others