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

Add Non-Maximum Merging (NMM) to Detections #500

Merged
merged 29 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c78ae33
feat: 🚀 Added Non-Maximum Merging to Detections
Oct 13, 2023
57b12e6
Added __setitem__ to Detections and refactored the object prediction …
Oct 18, 2023
9f22273
Added standard full image inference after sliced inference to increas…
Oct 18, 2023
6f47046
Refactored merging of Detection attributes to better work with np.nda…
Oct 18, 2023
5f0dcc2
Merge branch 'develop' into add_nmm_to_detections to resolve conflicts
Apr 9, 2024
166a8da
Implement Feedback
Apr 11, 2024
b159873
Merge remote-tracking branch 'upstream/develop' into add_nmm_to_detec…
May 6, 2024
d7e52be
NMM: Add None-checks, fix area normalization, style
May 6, 2024
bee3252
fix(pre_commit): 🎨 auto format pre-commit hooks
pre-commit-ci[bot] May 6, 2024
97c4071
NMM: Move detections merge into Detections class.
May 6, 2024
204669b
fix(pre_commit): 🎨 auto format pre-commit hooks
pre-commit-ci[bot] May 6, 2024
2eb0c7c
Merge remote-tracking branch 'upstream/develop' into add_nmm_to_detec…
LinasKo May 14, 2024
c3b77d0
Rename, remove functions, unit-test & change `merge_object_detection_…
May 14, 2024
8014e88
Test box_non_max_merge
May 14, 2024
26bafec
Test box_non_max_merge, rename threshold,to __init__
May 15, 2024
d2d50fb
renamed bbox -> xyxy
May 15, 2024
2d740bd
fix: merge_object_detection_pair
May 15, 2024
145b5fe
Rename to batch_box_non_max_merge to box_non_max_merge_batch
May 15, 2024
6c40935
box_non_max_merge: use our functions to compute iou
May 15, 2024
53f345e
Minor renaming
May 15, 2024
0e2eec0
Revert np.bool comparisons with `is`
May 15, 2024
559ef90
Simplify box_non_max_merge
May 15, 2024
f8f3647
Removed suprplus NMM code for 20% speedup
May 15, 2024
9024396
Add npt.NDarray[x] types, remove resolution_wh default val
May 17, 2024
6fbca83
Address review comments, simplify merge
May 23, 2024
db1b473
fix(pre_commit): 🎨 auto format pre-commit hooks
pre-commit-ci[bot] May 23, 2024
0721bc2
Remove _set_at_index
May 23, 2024
530e1d0
Address comments
May 27, 2024
2ee9e08
Renamed to group_overlapping_boxes
May 27, 2024
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
4 changes: 3 additions & 1 deletion supervision/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
DetectionDataset,
)
from supervision.detection.annotate import BoxAnnotator
from supervision.detection.core import Detections
from supervision.detection.core import Detections, merge_object_detection_pair
LinasKo marked this conversation as resolved.
Show resolved Hide resolved
from supervision.detection.line_zone import LineZone, LineZoneAnnotator
from supervision.detection.tools.csv_sink import CSVSink
from supervision.detection.tools.inference_slicer import InferenceSlicer
Expand All @@ -44,6 +44,8 @@
from supervision.detection.tools.smoother import DetectionsSmoother
from supervision.detection.utils import (
box_iou_batch,
box_non_max_merge,
LinasKo marked this conversation as resolved.
Show resolved Hide resolved
box_non_max_merge_batch,
box_non_max_suppression,
calculate_masks_centroids,
clip_boxes,
Expand Down
180 changes: 180 additions & 0 deletions supervision/detection/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

from supervision.config import CLASS_NAME_DATA_FIELD, ORIENTED_BOX_COORDINATES
from supervision.detection.utils import (
box_non_max_merge,
box_non_max_merge_batch,
box_non_max_suppression,
calculate_masks_centroids,
extract_ultralytics_masks,
Expand Down Expand Up @@ -1066,6 +1068,33 @@ def __setitem__(self, key: str, value: Union[np.ndarray, List]):

self.data[key] = value

def _set_at_index(self, index: int, other: Detections):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't placing this code as part of the setitem method makes more sense? The flow below feels quite natural to me.

detections_1 = sv.Detections(...)
detections_2 = sv.Detections(...)
detections_1[0] = detections_2[0]

Copy link
Collaborator

Choose a reason for hiding this comment

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

__setitam__

detections_2[0]
detections_2["class_name"]

__getitam__

detections_2[0]
detections_2[1:3]
detections_2[[1, 2, 3]]
detections_2[[False, True, False]]
detections_2["class_name"]

Copy link
Contributor

@LinasKo LinasKo May 23, 2024

Choose a reason for hiding this comment

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

_set_at_index was not required. I removed it entirely, and did not add any logic to __setitem__.

"""
Set detection values (xyxy, confidence, ...) at a specified index
to those of another Detections object, at index 0.

Args:
index (int): The index in current detection, where values
will be set.
other (Detections): Detections object with exactly one element
to set the values from.

Raises:
ValueError: If `other` is not made of exactly one element.
"""
if len(other) != 1:
raise ValueError("Detection to set from must have exactly one element.")

self.xyxy[index] = other.xyxy[0]
if self.mask is not None and other.mask is not None:
self.mask[index] = other.mask[0]
if self.confidence is not None and other.confidence is not None:
self.confidence[index] = other.confidence[0]
if self.class_id is not None and other.class_id is not None:
self.class_id[index] = other.class_id[0]
if self.tracker_id is not None and other.tracker_id is not None:
self.tracker_id[index] = other.tracker_id[0]

@property
def area(self) -> np.ndarray:
"""
Expand Down Expand Up @@ -1150,3 +1179,154 @@ def with_nms(
)

return self[indices]

def with_nmm(
self, threshold: float = 0.5, class_agnostic: bool = False
) -> Detections:
"""
Perform non-maximum merging on the current set of object detections.

Args:
threshold (float, optional): The intersection-over-union threshold
to use for non-maximum merging. Defaults to 0.5.
class_agnostic (bool, optional): Whether to perform class-agnostic
non-maximum merging. If True, the class_id of each detection
will be ignored. Defaults to False.

Returns:
Detections: A new Detections object containing the subset of detections
after non-maximum merging.

Raises:
AssertionError: If `confidence` is None and class_agnostic is False.
If `class_id` is None and class_agnostic is False.
"""
if len(self) == 0:
return self

assert 0.0 <= threshold <= 1.0, "Threshold must be between 0 and 1."

assert (
self.confidence is not None
), "Detections confidence must be given for NMM to be executed."

if class_agnostic:
predictions = np.hstack((self.xyxy, self.confidence.reshape(-1, 1)))
keep_to_merge_list = box_non_max_merge(predictions, threshold)
else:
assert self.class_id is not None, (
"Detections class_id must be given for NMS to be executed. If you"
" intended to perform class agnostic NMM set class_agnostic=True."
)
predictions = np.hstack(
(
self.xyxy,
self.confidence.reshape(-1, 1),
self.class_id.reshape(-1, 1),
)
)
keep_to_merge_list = box_non_max_merge_batch(predictions, threshold)

result = []
for keep_ind, merge_ind_list in keep_to_merge_list.items():
for merge_ind in merge_ind_list:
merged_detection = merge_object_detection_pair(
self[keep_ind], self[merge_ind]
)
self._set_at_index(keep_ind, merged_detection)
result.append(self[keep_ind])

return Detections.merge(result)


def merge_object_detection_pair(det1: Detections, det2: Detections) -> Detections:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please rename the arguments to detections_1 and detections_2.

Copy link
Collaborator

@SkalskiP SkalskiP May 21, 2024

Choose a reason for hiding this comment

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

In general, I think we have a naming problem. Our current marge should be called concatenate, and this should be just merge. But as long as merge_object_detection_pair is not part of public API we don't need to overthink it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I made an attempt to improve the naming:

  • box_non_max_merge -> _box_non_max_merge_all (open to name ideas)
  • box_non_max_merge_batch -> box_non_max_merge. Now it's the main method, exactly like * box_non_max_suppression
  • merge_object_detection_pair -> _merge_inner_detection_object_pair
  • new method: _merge_inner_detections_objects

"""
Merges two Detections object into a single Detections object.
Assumes each Detections contains exactly one object.

A `winning` detection is determined based on the confidence score of the two
input detections. This winning detection is then used to specify which
`class_id`, `tracker_id`, and `data` to include in the merged Detections object.

The resulting `confidence` of the merged object is calculated by the weighted
contribution of ea detection to the merged object.
The bounding boxes and masks of the two input detections are merged into a
single bounding box and mask, respectively.

Args:
det1 (Detections):
The first Detections object
det2 (Detections):
The second Detections object

Returns:
Detections: A new Detections object, with merged attributes.

Raises:
ValueError: If the input Detections objects do not have exactly 1 detected
object.

Example:
```python
import cv2
import supervision as sv
from inference import get_model

image = cv2.imread(<SOURCE_IMAGE_PATH>)
model = get_model(model_id="yolov8s-640")

result = model.infer(image)[0]
detections = sv.Detections.from_inference(result)

merged_detections = merge_object_detection_pair(
detections[0], detections[1])
```
"""
if len(det1) != 1 or len(det2) != 1:
raise ValueError("Both Detections should have exactly 1 detected object.")

if det2.confidence is None:
LinasKo marked this conversation as resolved.
Show resolved Hide resolved
winning_det = det1
elif det1.confidence is None:
winning_det = det2
elif det1.confidence[0] >= det2.confidence[0]:
winning_det = det1
else:
winning_det = det2

area_det1 = (det1.xyxy[0][2] - det1.xyxy[0][0]) * (
det1.xyxy[0][3] - det1.xyxy[0][1]
)
area_det2 = (det2.xyxy[0][2] - det2.xyxy[0][0]) * (
det2.xyxy[0][3] - det2.xyxy[0][1]
)

merged_x1, merged_y1 = np.minimum(det1.xyxy[0][:2], det2.xyxy[0][:2])
merged_x2, merged_y2 = np.maximum(det1.xyxy[0][2:], det2.xyxy[0][2:])
merged_xy = np.array([[merged_x1, merged_y1, merged_x2, merged_y2]])
LinasKo marked this conversation as resolved.
Show resolved Hide resolved

if det2.mask is None or det1.mask is None:
merged_mask = winning_det.mask
LinasKo marked this conversation as resolved.
Show resolved Hide resolved
else:
merged_mask = np.logical_or(det1.mask, det2.mask)

if det1.confidence is None or det2.confidence is None:
merged_confidence = winning_det.confidence
else:
merged_confidence = (
area_det1 * det1.confidence[0] + area_det2 * det2.confidence[0]
) / (area_det1 + area_det2)
merged_confidence = np.array([merged_confidence])

winning_class_id = winning_det.class_id
winning_tracker_id = winning_det.tracker_id
winning_data = winning_det.data
LinasKo marked this conversation as resolved.
Show resolved Hide resolved

return Detections(
xyxy=merged_xy,
mask=merged_mask,
confidence=merged_confidence,
class_id=winning_class_id,
tracker_id=winning_tracker_id,
data=winning_data,
)
Loading