Feat/core/bytetrack #40
Conversation
AlexBodner
requested review from
soumik12345,
SkalskiP and
onuralpszr
as code owners
…ckers into feat/core/bytetrack
|
Hi @onuralpszr — any chance you could take a look at these MyPy issues? They actually originate from @soumik12345’s PR: #49. Your MyPy checks were merged in parallel, so this is the first PR where both changes are part of the same codebase. I took a look at the errors but couldn’t figure out how to resolve them. |
Sure let me take a look. |
…t function and only accept int
…in TripletsDataset
| @@ -28,7 +34,7 @@ class TripletsDataset(Dataset): | |||
| Attributes: | |||
| tracker_id_to_images (dict[str, list[str]]): Dictionary mapping tracker IDs | |||
| to lists of image paths | |||
| transforms (Optional[Compose]): Optional image transformations to apply | |||
| transforms (Union[Compose, ToTensor]): Image transformations to apply | |||
There was a problem hiding this comment.
@onuralpszr what was the reason for making this change? The docstring no longer matches the actual type of the argument it describes.
There was a problem hiding this comment.
It does match and Previous code already enforcing "Compose, ToTensor" in transforms I just added type annotation
And for the function side you return "Tensor" and I made sure it is Tensor (which is I transformed) but IF* it is actually "Optional" I need to change small bit
| if not hasattr(self.backbone_model, "num_features"): | ||
| raise AttributeError( | ||
| "Backbone model is missing 'num_features' attribute, " | ||
| "cannot add projection layer." | ||
| ) | ||
|
|
||
| num_model_features = getattr(self.backbone_model, "num_features") | ||
| if not isinstance(num_model_features, int): | ||
| raise TypeError( | ||
| f"Backbone model's 'num_features' must be an int, " | ||
| f"but got {type(num_model_features)}." | ||
| ) | ||
|
|
There was a problem hiding this comment.
@onuralpszr could you explain why this was needed and how it affected the MyPy check results?
There was a problem hiding this comment.
This part I wanted to be ensure we have that parameter (not related to mypy but cover the missing attribute error)
if not hasattr(self.backbone_model, "num_features"):
raise AttributeError(
"Backbone model is missing 'num_features' attribute, "
"cannot add projection layer."
)Mypy part is here (we are not enforcing any so it is fine)
num_model_features = getattr(self.backbone_model, "num_features")Here since I am getting any it will work but I wanted to make sure for "int" value so I added a check
if not isinstance(num_model_features, int):
raise TypeError(
f"Backbone model's 'num_features' must be an int, "
f"but got {type(num_model_features)}."
)
| @@ -38,7 +44,7 @@ def __init__( | |||
| transforms: Optional[Compose] = None, | |||
| ): | |||
| self.tracker_id_to_images = validate_tracker_id_to_images(tracker_id_to_images) | |||
| self.transforms = transforms or ToTensor() | |||
| self.transforms: Union[Compose, ToTensor] = transforms or ToTensor() | |||
There was a problem hiding this comment.
| self.transforms: Union[Compose, ToTensor] = transforms or ToTensor() | |
| self.transforms: Compose = transforms or transforms.Compose([ToTensor()]) |
@soumik12345 would it make sense to update this line like this? It might help unify the MyPy typing in this part of the code.
There was a problem hiding this comment.
I think a better type hint would be Union[Compose, Callable] because any stateful callables should technically be possible to be composed as transforms.
…ckers into feat/core/bytetrack
fix(trackers): bytetrack feature association
Description
This pull request adds the ByteTrack tracker to the list of implemented trackers.
No extra depedencies are required for usage, for testing I used pytest which is in the dev dependencies. The test file can be removed if you consider so, let me know.
Type of change
How has this change been tested, please provide a testcase or example of how you tested the change?
The implementation has been thoroughly tested, passing all the evaluations, and showing the expected results.
The new tracker can be used in the following Google Colab, which shows 2 different examples of the Tracker in its 2 possible modes: using appearance features for high confidence boxes and using IoU.
There is also a test file in the route: trackers/core/bytetrack/tests_bytetrack.py, this tests cover the main functioning of the tracker and check that it behaves as expected.
Any specific deployment considerations
Check trackers/init.py so that it can be imported correctly.
I've updated my trackers/core/deepsort/feature_extractor.py to be exactly like the one in the main branch, but for a reason it compares it to the old one.