Skip to content

fix(perception_fp): change method contain to intersect #333

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.

Sign up for GitHub

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

Open
MasatoSaeki wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix(perception_fp): change method contain to intersect #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@

class NonDetectionArea(BaseModel):
frame_id: Literal["map", "base_link"]
polygon: list[conlist(number, min_length=3, max_length=3)]

Check failure on line 140 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this unexpected named argument 'min_length'. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIm&open=AZzb-C4dkZa_-9vKhRIm&pullRequest=333

Check failure on line 140 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this unexpected named argument 'max_length'. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIn&open=AZzb-C4dkZa_-9vKhRIn&pullRequest=333
z_min: number | None = None
z_max: number | None = None

Expand Down Expand Up @@ -187,7 +187,7 @@
criteria_name: str | None = None
PassRate: number
non_detection_area: NonDetectionArea
timestamp: list[TimeStamp] | None

Check failure on line 190 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Add an explicit default value to this optional field. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIo&open=AZzb-C4dkZa_-9vKhRIo&pullRequest=333
topic_type: list[Literal["bbox", "pointcloud"]]

def get_type(self) -> tuple[type, ...]:
Expand Down Expand Up @@ -224,11 +224,11 @@

def set_frame(
self,
timestamp: float,

Check failure on line 227 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter timestamp or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIp&open=AZzb-C4dkZa_-9vKhRIp&pullRequest=333
frame_id: str,

Check failure on line 228 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter frame_id or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIq&open=AZzb-C4dkZa_-9vKhRIq&pullRequest=333
data: PerceptionFPData,

Check failure on line 229 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter data or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIr&open=AZzb-C4dkZa_-9vKhRIr&pullRequest=333
map_to_base_link: np.ndarray,

Check failure on line 230 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter map_to_base_link or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIs&open=AZzb-C4dkZa_-9vKhRIs&pullRequest=333
base_link_to_map: np.ndarray,

Check failure on line 231 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter base_link_to_map or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIt&open=AZzb-C4dkZa_-9vKhRIt&pullRequest=333
) -> dict | None:
if self.condition.timestamp is not None and not any(
condition_timestamp.is_valid(timestamp)
Expand Down Expand Up @@ -277,7 +277,7 @@
},
}

def is_in_non_detection_area(

Check failure on line 280 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIu&open=AZzb-C4dkZa_-9vKhRIu&pullRequest=333
self,
frame_id: str,
data: PerceptionFPData,
Expand Down Expand Up @@ -308,11 +308,7 @@
else True
)
)
is_in_xy = np.any(
contains(
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])))
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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
)

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
is_in_xy = np.any(prep_geom.intersects(Polygon(corners[:, 0:2])))
is_in_xy = prep_geom.intersects(Polygon(corners[:, 0:2]))

Copilot uses AI. Check for mistakes.
if is_in_z and is_in_xy:
Comment on lines +311 to 312
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
fp.append(bbox)
exist = len(fp) != 0
Expand All @@ -336,7 +332,7 @@
]
exist = pointcloud_in_non_detection_area.size != 0
fp_objects = pointcloud_in_non_detection_area
# TODO: consider better way to store debug objects

Check warning on line 335 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Complete the task associated to this "TODO" comment. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIv&open=AZzb-C4dkZa_-9vKhRIv&pullRequest=333
self._non_detection_area = non_detection_area
self._fp_objects = fp_objects
return exist
Expand All @@ -359,7 +355,7 @@
z_max=self.condition.non_detection_area.z_max,
)
if frame_id == non_detection_area.frame_id:
pass

Check warning on line 358 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Either remove or fill this block of code. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIw&open=AZzb-C4dkZa_-9vKhRIw&pullRequest=333
elif frame_id == "base_link" and non_detection_area.frame_id == "map":
non_detection_area.convert_map_to_base_link()
elif frame_id == "map" and non_detection_area.frame_id == "base_link":
Expand Down Expand Up @@ -401,12 +397,12 @@

def set_frame(
self,
timestamp: float,

Check failure on line 400 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter timestamp or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIx&open=AZzb-C4dkZa_-9vKhRIx&pullRequest=333
frame_id: str,

Check failure on line 401 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter frame_id or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIy&open=AZzb-C4dkZa_-9vKhRIy&pullRequest=333
data: PerceptionFPData,

Check failure on line 402 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter data or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRIz&open=AZzb-C4dkZa_-9vKhRIz&pullRequest=333
skip: int,

Check failure on line 403 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter skip or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRI0&open=AZzb-C4dkZa_-9vKhRI0&pullRequest=333
map_to_base_link: np.ndarray,

Check failure on line 404 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter map_to_base_link or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4dkZa_-9vKhRI1&open=AZzb-C4dkZa_-9vKhRI1&pullRequest=333
base_link_to_map: np.ndarray,

Check failure on line 405 in driving_log_replayer_v2/driving_log_replayer_v2/perception_fp/models.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove parameter base_link_to_map or provide default value. 


See more on https://sonarcloud.io/project/issues?id=tier4_driving_log_replayer_v2&issues=AZzb-C4ekZa_-9vKhRI2&open=AZzb-C4ekZa_-9vKhRI2&pullRequest=333
) -> None:
self._frame = {"FrameSkip": skip}
for criterion in self.__perception_fp_criterion:
Expand Down
13 changes: 13 additions & 0 deletions sample/perception_fp/scenario.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,16 @@ Evaluation:
z_max: null # [m] null [Do not filter by z_max] or value
timestamp: null # null [Do not filter by timestamp] or ros timestamp value in seconds
topic_type: [pointcloud] # List of topic types to evaluate
- PassRate: 99.0
criteria_name: test_function
non_detection_area: # Non-detection area specification in map coordinates
frame_id: map
polygon: # List of vertices of the polygon in map coordinates (x, y, z), clockwise order
- [3806.4705, 73721.0869, 19.6891]
- [3804.568, 73709.4626, 19.7295]
- [3799.1703, 73706.7849, 19.6921]
- [3804.503, 73720.9015, 19.6713]
z_min: null # [m] null [Do not filter by z_min] or value
z_max: null # [m] null [Do not filter by z_max] or value
timestamp: null # null [Do not filter by timestamp] or ros timestamp value in seconds
topic_type: [bbox] # List of topic types to evaluate
Loading