Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New vectorized distance functions. #211

Merged
merged 5 commits into from Nov 18, 2022

Conversation

facundo-lezama
Copy link
Collaborator

@facundo-lezama facundo-lezama commented Nov 1, 2022

In this PR we add support for vectorized distance_functions to speed up Norfair. The new distances can be one of ScalarDistance, VectorizedDistance, and ScipyDistance. We moved the distance functions and the distances calculations (previously Tracker._get_distances) to the newly mentioned classes. With this new interface, we can define a variety of distances; from simple scalar distances such as the ones that we have been using until now, to custom vectorized distances that take a pair of np.ndarray as input, and even default vectorized distances from Scipy.

We were careful not to break the API. The users can still define custom distances and now they can also leverage predefined vectorized distances that speed up the tracking.

Changes include:

  • Defined new distance classes in distances.py.
  • Removed the Tracker._get_distances and moved it to the distance classes.
  • Adapted Tracker.__init__ to create instances of the new distance classes.
  • Adapted demos to use vectorized distances when possible.
  • Adapted tests and include new ones for the new distances interface.
  • Updated deprecated documentation in README.

Pending tasks:
[ x ] Change demos to use vectorized distances when possible
[ x ] Check ReID still works
[ x ] Add tests
[ x ] Review and update any deprecated documentation
[ x ] Write PR description

@facundo-lezama facundo-lezama force-pushed the vectorized-distance-functions branch 9 times, most recently from 2549262 to 0815f94 Compare November 3, 2022 17:03
@facundo-lezama facundo-lezama marked this pull request as ready for review November 3, 2022 17:29
README.md Outdated Show resolved Hide resolved
demos/alphapose/writer.py Show resolved Hide resolved
demos/yolov4/Dockerfile Outdated Show resolved Hide resolved
norfair/distances.py Outdated Show resolved Hide resolved
norfair/tracker.py Outdated Show resolved Hide resolved
norfair/distances.py Show resolved Hide resolved
norfair/distances.py Outdated Show resolved Hide resolved
norfair/distances.py Outdated Show resolved Hide resolved
norfair/distances.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@javiber javiber left a comment

Choose a reason for hiding this comment

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

LGTM. please wait for #212 to be merged and the new release before merging this.
I would like to make a small patch-release and I feel this one is too big a change to be included in a patch

@dekked
Copy link
Member

dekked commented Nov 17, 2022

Why do we keep both iou and iou_opt?

What use case does the unoptimized version have?

@facundo-lezama
Copy link
Collaborator Author

Why do we keep both iou and iou_opt?

What use case does the unoptimized version have?

@dekked the unoptimized version has some validations that might be useful. This PR doesn't address the modification of the IoU distance function as a vectorized distance, that's why we don't modify it here. In #213 we are changing the optimized IoU implementation, and we will revisit these two definitions.

@facundo-lezama facundo-lezama merged commit 2b344e3 into master Nov 18, 2022
@facundo-lezama facundo-lezama deleted the vectorized-distance-functions branch November 18, 2022 17:43
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.

None yet

3 participants