Skip to content

Conversation

@Aaryan2304
Copy link
Contributor

Summary

This PR fixes a side-effect in ByteTrackTracker.update() where the input sv.Detections object was modified in-place, overwriting existing tracker_id values with -1. This behavior caused data loss for pipelines that reuse the detections object after the tracking step.

Changes

  • Removed redundant in-place initialization: detections.tracker_id = -np.ones(len(detections)) (line 111)
    • This was unnecessary because all outputs are built via deepcopy() calls in _update_detections(), the unmatched loop, and _spawn_new_trackers().
  • Updated the empty tracks/detections edge case (line 103-104) to return a deepcopy() instead of mutating and returning the input directly.

Verification

  • All ruff checks pass (linting and formatting)
  • Type checking (mypy) passes with no errors
  • Verified locally that the input detections object retains its original values after calling update()
  • Confirmed that returned tracking results are correct and contain expected IDs

CLA Signing

I have read the CLA Document and I sign the CLA.

Copilot AI review requested due to automatic review settings January 29, 2026 16:05
@Aaryan2304 Aaryan2304 requested a review from SkalskiP as a code owner January 29, 2026 16:05
@CLAassistant
Copy link

CLAassistant commented Jan 29, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a side-effect bug in ByteTrackTracker.update() where the input sv.Detections object was being modified in-place, causing data loss for downstream code that reuses the detections object. The fix ensures the input remains immutable by using deepcopy() for all return paths.

Changes:

  • Removed redundant in-place mutation of input detections (detections.tracker_id = -np.ones(len(detections)))
  • Fixed edge case handling to return a deep copy instead of mutating the input when both tracks and detections are empty

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Borda Borda added the bug Something isn't working label Feb 5, 2026
@SkalskiP SkalskiP self-assigned this Feb 11, 2026
@SkalskiP
Copy link
Collaborator

Hi @Aaryan2304 👋🏻 Thanks a lot for your interes in trackers and for submitting this PR. We are planning trackers-2.2.0 release for this week and I'll make sure we will include this fix.

@Aaryan2304
Copy link
Contributor Author

Hi @Aaryan2304 👋🏻 Thanks a lot for your interes in trackers and for submitting this PR. We are planning trackers-2.2.0 release for this week and I'll make sure we will include this fix.

Thanks @SkalskiP ! Appreciate you including this in the next release.
Let me know if you need any changes to the PR before then.

@SkalskiP
Copy link
Collaborator

Hi @Aaryan2304 👋🏻

It looks like the same bug is also partially present in SORT.

trackers/core/sort/tracker.py lines 133-135

if len(self.trackers) == 0 and len(detections) == 0:
    detections.tracker_id = np.array([], dtype=int)
    return detections

trackers/core/bytetrack/tracker.py lines 112-114

if len(self.tracks) == 0 and len(detections) == 0:
    detections.tracker_id = np.array([], dtype=int)
    return detections

@AlexBodner
Copy link
Collaborator

@SkalskiP ran soccernet benchmark over this branch and gives same result + from my overview seems like an adequate change

@SkalskiP
Copy link
Collaborator

@Aaryan2304 I'm merging the PR. Thanks a lot for your contribution! We may need similar PR for SORT.

@SkalskiP SkalskiP merged commit 1549682 into roboflow:develop Feb 11, 2026
15 checks passed
@Aaryan2304
Copy link
Contributor Author

@Aaryan2304 I'm merging the PR. Thanks a lot for your contribution! We may need similar PR for SORT.

Ok, I'll make the necessary changes and raise a PR accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants