Conversation
Bugfix in getting the original det ID back. Also, make sure active tracks objects / observations all have the same dimensions
There was a problem hiding this comment.
Pull Request Overview
Introduces a new HybridSort tracker and integrates it into configuration/tests, rewrites its tracking logic (Kalman filter, association, ReID handling), and updates logging initialization (richer log formatting, per-process setup). Also modifies association utilities and adds process pool worker logging setup.
- Add HybridSort to tracker registries and per-class lists
- Replace previous HybridSort implementation with a substantially refactored version (KF variants, association flow, ReID/ECC integration)
- Revise logging configuration and exception handling; extend association utilities with new helpers and hard‑coded weights
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_config.py | Registers HybridSort across tracker name lists (including per-class). |
| boxmot/utils/init.py | Replaces logger.configure with explicit logger.add removing main-process filter; returns logger. |
| boxmot/trackers/hybridsort/hybridsort.py | Large rewrite of HybridSort (KF initialization, update/predict logic, output formatting, association pipeline). |
| boxmot/trackers/hybridsort/association.py | Style changes, added functions, hard-coded weights replacing parameters, deprecated dtype usage, unused import. |
| boxmot/engine/val.py | Improves exception logging and ensures per-process logging reinitialization in workers. |
Comments suppressed due to low confidence (1)
boxmot/trackers/hybridsort/hybridsort.py:56
- Accessing bbox[4] assumes a 5-element input; if a 4-element box is passed (as implied by comment and prior behavior) this raises IndexError. Additionally, using 'if score:' drops valid zero scores and shifts state layout (x[3] becomes r), causing downstream misinterpretation (e.g., predict() treating x[3] as score). Use length check instead: if len(bbox) == 5: ...; always preserve consistent state shape.
# [x1,y1,x2,y2,score] -> [x,y,s,score,r] or [x,y,s,r]
w = bbox[2] - bbox[0]
h = bbox[3] - bbox[1]
x = bbox[0] + w / 2.0
y = bbox[1] + h / 2.0
s = w * h
r = w / float(h + 1e-6)
score = bbox[4]
if score:
return np.array([x, y, s, score, r]).reshape((5, 1))
else:
return np.array([x, y, s, r]).reshape((4, 1))
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| self.confidence = bbox[4] | ||
| self.confidence = float(bbox[-1]) | ||
| else: | ||
| self.kf.update(bbox) |
There was a problem hiding this comment.
Calling self.kf.update(None) (bbox is None in this branch) will raise or corrupt the filter; the 'no observation' case should skip update and only advance predict counters. Replace with a conditional that does not call update when bbox is None.
| self.kf.update(bbox) | |
| # No observation: skip Kalman update, only advance counters/state as needed |
| if not self.confidence_pre: | ||
| return ( | ||
| self.history[-1], | ||
| np.clip(self.kf.x[3], track_thresh, 1.0), | ||
| np.clip(self.confidence, 0.1, track_thresh), | ||
| ) | ||
| simple_score = float(np.clip(self.confidence, 0.1, self.track_thresh)) | ||
| else: | ||
| return ( | ||
| self.history[-1], | ||
| np.clip(self.kf.x[3], track_thresh, 1.0), | ||
| np.clip(self.confidence - (self.confidence_pre - self.confidence), 0.1, track_thresh) | ||
| ) | ||
| simple_score = float(np.clip( | ||
| self.confidence - (self.confidence_pre - self.confidence), | ||
| 0.1, | ||
| self.track_thresh, | ||
| )) |
There was a problem hiding this comment.
The 'if not self.confidence_pre' test fails when previous confidence is 0.0 (valid value) causing logic to treat it as None. Use 'if self.confidence_pre is None:' instead to distinguish unset from zero.
| KalmanBoxTracker.count += 1 | ||
| self.max_obs = max_obs | ||
| self.history = deque([], maxlen=self.max_obs) | ||
| self.history: List[np.ndarray] = [] |
There was a problem hiding this comment.
Replaced bounded deques with plain lists removing maxlen constraints; over long runs this can accumulate unbounded history and increase memory usage. Restore deque(maxlen=...) or enforce a cap when appending.
| self.history: List[np.ndarray] = [] | |
| self.history: deque[np.ndarray] = deque(maxlen=30) |
| self.last_observation_save = np.array([-1, -1, -1, -1, -1]) | ||
| self.observations = dict() | ||
| self.history_observations = deque([], maxlen=self.max_obs) | ||
| self.history_observations: List[np.ndarray] = [] |
There was a problem hiding this comment.
Replaced bounded deques with plain lists removing maxlen constraints; over long runs this can accumulate unbounded history and increase memory usage. Restore deque(maxlen=...) or enforce a cap when appending.
| self.history_observations: List[np.ndarray] = [] | |
| self.history_observations = deque([], maxlen=int(longterm_bank_length)) |
| if (trk.time_since_update < 1) and (trk.hit_streak >= self.min_hits or self.frame_count <= self.min_hits): | ||
| outputs.append([*d.tolist(), trk.id + 1, trk.confidence, 0, -1]) # cls=0 default, det_ind=-1 (not tracked here) | ||
|
|
There was a problem hiding this comment.
Class ID is hard-coded to 0 and detection index to -1, yet HybridSort was added to PER_CLASS_TRACKERS; this breaks per-class tracking expectations and downstream class-based metrics. Populate cls and det_ind from the original detection (store them when creating/updating the track).
|
|
||
| # TCM | ||
| angle_diff_cost -= score_dif * TCM_first_step_weight | ||
| angle_diff_cost -= score_dif * 0.6 |
There was a problem hiding this comment.
Same hard-coded weight issue as earlier; this second occurrence also needs re-parametrization to avoid locking in an arbitrary constant and breaking prior configuration semantics.
| cost_matrix = np.zeros((len(tracks_feat), len(detections_feat)), dtype=np.float) | ||
| if cost_matrix.size == 0: | ||
| return cost_matrix | ||
| # det_features = np.asarray([track.curr_feat for track in detections], dtype=np.float64) # [detection_num, emd_dim] | ||
| # det_features = np.asarray([track.curr_feat for track in detections], dtype=np.float) # [detection_num, emd_dim] | ||
| # #for i, track in enumerate(tracks): | ||
| # #cost_matrix[i, :] = np.maximum(0.0, cdist(track.smooth_feat.reshape(1,-1), det_features, metric)) | ||
| # track_features = np.asarray([track.smooth_feat for track in tracks], dtype=np.float64) # [track_num, emd_dim] | ||
| # Nomalized features, metric: cosine, [track_num, detection_num] | ||
| cost_matrix = np.maximum(0.0, cdist(tracks_feat, detections_feat, metric)) | ||
| # track_features = np.asarray([track.smooth_feat for track in tracks], dtype=np.float) # [track_num, emd_dim] |
There was a problem hiding this comment.
Uses deprecated 'np.float' (removed in NumPy 2.0) which will raise an AttributeError; replace with float or np.float64 to ensure forward compatibility.
| cost_matrix = np.zeros((len(tracks_feat), len(detections_feat)), dtype=np.float) | ||
| if cost_matrix.size == 0: | ||
| return cost_matrix | ||
| # det_features = np.asarray([track.curr_feat for track in detections], dtype=np.float64) # [detection_num, emd_dim] | ||
| # det_features = np.asarray([track.curr_feat for track in detections], dtype=np.float) # [detection_num, emd_dim] | ||
| # #for i, track in enumerate(tracks): | ||
| # #cost_matrix[i, :] = np.maximum(0.0, cdist(track.smooth_feat.reshape(1,-1), det_features, metric)) | ||
| # track_features = np.asarray([track.smooth_feat for track in tracks], dtype=np.float64) # [track_num, emd_dim] | ||
| # Nomalized features, metric: cosine, [track_num, detection_num] | ||
| cost_matrix = np.maximum(0.0, cdist(tracks_feat, detections_feat, metric)) | ||
| # track_features = np.asarray([track.smooth_feat for track in tracks], dtype=np.float) # [track_num, emd_dim] |
There was a problem hiding this comment.
Uses deprecated 'np.float' (removed in NumPy 2.0) which will raise an AttributeError; replace with float or np.float64 to ensure forward compatibility.
| def update(self, bbox, id_feature, update_feature: bool = True): | ||
| vlt = vrt = vlb = vrb = None |
There was a problem hiding this comment.
update() signature dropped 'cls' and 'det_ind' but downstream output logic (and per-class tracking expectations) rely on class/index retention; also convert_bbox_to_z here inherits earlier shape issues which may mismatch kf.dim_z (4 vs 5) depending on use_custom_kf. Preserve and store class/index fields and ensure measurement dimensionality matches the KalmanFilter's configured dim_z.
| self.hits += 1 | ||
| self.hit_streak += 1 | ||
| self.kf.update(convert_bbox_to_z(bbox)) |
There was a problem hiding this comment.
update() signature dropped 'cls' and 'det_ind' but downstream output logic (and per-class tracking expectations) rely on class/index retention; also convert_bbox_to_z here inherits earlier shape issues which may mismatch kf.dim_z (4 vs 5) depending on use_custom_kf. Preserve and store class/index fields and ensure measurement dimensionality matches the KalmanFilter's configured dim_z.
* Update hybridsort.py Bugfix in getting the original det ID back. Also, make sure active tracks objects / observations all have the same dimensions * Added HybridSort back to test_config.py * more detailed logs * fix * add missing files * add hybridsort to ci jobs * adapt to MOT17 * fix benchmarking job * add hybridsort * add hybridsort --------- Co-authored-by: Hotte <hobenhaus@gmail.com>
No description provided.