fix enumerate call

[rolson24]

Description

I made a very silly mistake in my PR #1035. The enumerate returns and index and a tuple and I was unpacking that tuple incorrectly. I have since re-tested it with this change, and it works correctly now. I also had to fix a bug when there are no existing track_ids.

List any dependencies that are required for this change.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How has this change been tested, please provide a testcase or example of how you tested the change?

This colab notebook https://colab.research.google.com/drive/1azgc_A-divM3Z5l0TyHW2SRHnw6EUlbO#scrollTo=J-lQUq30tUgD is what I used to test it.

[SkalskiP]

Can tracker_id be None?

[SkalskiP]

Hi @rolson24 👋🏻 I understand for i, (idet, itrack) in enumerate(matches): but what is the reason for rest of the changes?

[rolson24]

There was a second bug that I found. In my original testing I was accidentally using model.track(frame) to get the detections from ultralytics. Then when I called sv.from_ultralytics each detection would already have a tracker ID so the supervision Detection class would have a valid track_id. When I tested the change with just using model (frame), the track_id array would be initialized to ‘None’ instead of a ‘np.array’. This means that when I tried to assign an index of the track_id array to the new track id in update_with_detections(), the code would throw an error. I fixed this by initializing the track_id variable to a np.array

…

On Tue, Mar 26, 2024 at 6:50 AM Piotr Skalski ***@***.***> wrote: Hi @rolson24 <https://github.com/rolson24> 👋🏻 I understand for i, (idet, itrack) in enumerate(matches): but what is the reason for rest of the changes? — Reply to this email directly, view it on GitHub <#1049 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AX2EJPGZ3O3VPBC6DWP53LTY2FAGFAVCNFSM6AAAAABFIBPAWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRQGEYDKMJSGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[SkalskiP]

Hi @rolson24 👋🏻 I wonder if we didn't overcomplicate the update_with_detections implementation. Take a look at the code below. What do you think?

def update_with_detections(self, detections: Detections) -> Detections:
    tensors = detections2boxes(detections=detections)
    tracks = self.update_with_tensors(tensors=tensors)

    if len(tracks) > 0:
        detection_bounding_boxes = np.asarray([det[:4] for det in tensors])
        track_bounding_boxes = np.asarray([track.tlbr for track in tracks])

        ious = box_iou_batch(detection_bounding_boxes, track_bounding_boxes)
        iou_costs = 1 - ious

        matches, _, _ = matching.linear_assignment(iou_costs, 0.5)
        detections.tracker_id = np.full(len(detections), -1, dtype=int)

        for i_detection, i_track in enumerate(matches):
            detections.tracker_id[i_detection] = int(tracks[i_track].track_id)

        return detections[detections.tracker_id != -1]

    else:
        detections.tracker_id = np.array([], dtype=int)
        return detections

[rolson24]

Oh, that's a good point. It would fix both the issue of returning detections that never actually are tracked, and the minimum_consecutive_frames arguement. Would you like me to add this in a new branch from the current develop branch?

[SkalskiP]

@rolson24

1. I think we should solve this issue first and then merge the minimum_consecutive_frames PR. Let's keep those two separate.
2. I'd like to ask you to thoroughly test if we do not introduce any bugs with that change. I'm always super worried about making any changes in trackers.

[rolson24]

Hi @SkalskiP

I agree with both of those, I updated my colab notebook to have more comprehensive tests, which should cover all of the different scenarios of detection objects being given to the tracker.

[SkalskiP]

Hi @rolson24 👋🏻 I left a comment regarding:

invalid_tracks = [
    i
    for i in range(len(detections.tracker_id))
    if detections.tracker_id[i] != -1
]

return detections[invalid_tracks]

[SkalskiP]

@rolson24 I assume that after #1062 we can change this into return detections[detections.tracker_id != =1] ?

[rolson24]

Yup!

[rolson24]

Also, I am wondering if we should add some unit tests for ByteTrack. I have some rudimentary one in a colab notebook. It would help when people need to test changes to ByteTrack and prevent bugs like this from happening again.

[SkalskiP]

@rolson24 I'm merging this PR. As for ByteTrack unit tests, I would love this! Feel free to open the next PR with tests.

[rolson24]

This is actually wrong. I forgot to test with low-confidence detections, and when that happens, the tracker_ids are -1, but the size of tracks returned from update_with_detections is none-zero, so it actually returns tracks that aren't actually tracked, but just with no track_id. I will submit a new PR with the needed change and will make sure the tests cover all of the possible cases.