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

Add optimized IoU function #226

Merged
merged 5 commits into from
Jan 3, 2023
Merged

Add optimized IoU function #226

merged 5 commits into from
Jan 3, 2023

Conversation

facundo-lezama
Copy link
Collaborator

@facundo-lezama facundo-lezama commented Dec 12, 2022

This PR adds a new implementation for the IoU distance calculation by leveraging numpy for vectorized calculations.

Changes include:

  • Implementation of a vectorized IoU
  • Modification of tests accordingly
  • Adapt demos to remove deprecated iou_opt

NOTE: We also implemented an optimized IoU function in Cython at #213, but after profiling both implementations, we saw that the performance was almost the same. Thus we decided to use the one introduced in this PR due to its simplicity.

Pending:

  • Test on some demos

to be in `[x_min, y_min, x_max, y_max]` format.

Normal IoU is 1 when the boxes are the same and 0 when they don't overlap,
to transform that into a distance that makes sense we return `1 - iou`.

Performs faster but errors might be cryptic if the bounding boxes are not valid.
Copy link
Member

Choose a reason for hiding this comment

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

Should we add some checks?

@facundo-lezama facundo-lezama force-pushed the numpy-optimized-iou branch 3 times, most recently from d905f71 to e1df7a5 Compare December 13, 2022 18:31
@facundo-lezama facundo-lezama marked this pull request as ready for review December 13, 2022 19:06
@dekked dekked requested a review from javiber December 19, 2022 18:31
The distance.
"""
return _iou(detection.points, tracked_object.estimate)
iou_opt = iou # deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Is it still necessary with the change in line 415?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did this so that users can still import iou_opt as a function, trying not to make that many breaking changes.

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 but I have a question regarding the tests

np.testing.assert_almost_equal(iou.distance_function(det, obj), 0)
np.testing.assert_almost_equal(iou_opt.distance_function(det, obj), 0)
det.points = det.points.reshape(1, 4)
obj.estimate = obj.estimate.reshape(1, 4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is the reshaping necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As we discussed, I simplified these.

@javiber javiber force-pushed the numpy-optimized-iou branch from 02af4c5 to c316914 Compare January 3, 2023 17:27
Also enables kwargs to be passed to scipy's cdist
@javiber javiber merged commit f766aba into master Jan 3, 2023
@javiber javiber deleted the numpy-optimized-iou branch January 3, 2023 18:02
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.

4 participants