Skip to content

Conversation

onuralpszr
Copy link
Contributor

@onuralpszr onuralpszr commented Apr 15, 2025

Fixed errors are

trackers/core/deepsort/feature_extractor.py:116: error: Item "None" of "Any | None" has no attribute "to"  [union-attr]
trackers/core/deepsort/feature_extractor.py:117: error: Item "None" of "Any | None" has no attribute "eval"  [union-attr]
trackers/core/deepsort/feature_extractor.py:154: error: "None" not callable  [misc]
trackers/core/deepsort/tracker.py:170: error: Missing positional argument "model_or_checkpoint_path" in call to "DeepSORTFeatureExtractor"  [call-arg]
Found 4 errors in 2 files (checked 14 source files)

raise ValueError(
"model_or_checkpoint_path cannot be None. "
"Please provide a valid model instance, file path, or URL."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, having allowing a None value only makes sense if we can set a default model instead of raising a ValueError.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@soumik12345 which model ? :) I didn't wanna slap resnet50 or something please let me know which :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, a reproduction of the original feature extractor architecture described in Section 2.4 and Table 1 of the DeepSORT paper should be treated as a default model (downloaded from a default URL). However, since the paper doesn't reveal the training methodology of the model, we will need to research a little bit regarding what might be the best way to reproduce the results from the paper.

Copy link
Contributor

Choose a reason for hiding this comment

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

@SkalskiP I would like to know your thoughts on this as well.

Copy link

@rolson24 rolson24 Apr 15, 2025

Choose a reason for hiding this comment

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

@onuralpszr Not sure that would follow the clean-room implementation goals of this project, as the DeepSORT code is not open license.

Alternatively, we could also use models from Fast-Reid which is Apache 2.0. The models from Fast-ReId are still being used in SOTA papers. For example DeepOC-SORT uses SBS50 from Fast-ReId:
"Implementations Our implementation is based on OCSORT [2, 28]. We use the same YOLOX detector as recent
works [4, 3, 13, 1] to make a fair comparison of tracking performance. For Re-ID, we use SBS50 from the fast-reid [29] library."
(DeepOC-SORT paper)

The DeepOC-SORT paper appears to use the ReID model to extract the features for each detection, similar to our implementation of DeepSORT:
"In previous work [3, 4], the deep visual embedding used to describe a tracklet is given by an Exponential Moving Average (EMA) of the deep detection embeddings frame by frame." (DeepOC-SORT paper)

Alternatively, we could reproduce the DeepSORT architecture described in the paper, and train on Market-1501, which doesn't seem to have a license (couldn't find one?) and is widely available on Kaggle.

Other ReID datasets have academic only licenses, but it seems like some ReID models trained on them are open-license (probably want to avoid this)? Some datasets have also been retracted due to privacy issues

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @soumik12345's point here:

allowing a None value only makes sense if we can set a default model instead of raising a ValueError.

I'll be committing my related updates to this PR shortly. As @soumik12345 noted, while our long-term goal is to publish our own fine-tuned weights, the current default pathway involves initializing the feature extractor with a timm model, like so:

from trackers import DeepSORTTracker, DeepSORTFeatureExtractor

feature_extractor = DeepSORTFeatureExtractor.from_timm("resnet50d.ra2_in1k")
tracker = DeepSORTTracker(feature_extractor)

While this initialization is slightly verbose, we intend to simplify it ahead of the v2.0.0 release.

@CLAassistant
Copy link

CLAassistant commented Apr 15, 2025

CLA assistant check
All committers have signed the CLA.

@onuralpszr onuralpszr merged commit a5ca735 into main Apr 16, 2025
18 checks passed
@soumik12345 soumik12345 deleted the mypy/fixes branch May 1, 2025 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants