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

feat: Add soft Non-Max suppression #1624

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

YHallouard
Copy link

Description

Hi everyone,

I am opening this PR to propose an implementation of Soft NMS based on that paper.

I propose a method with_soft_nms, but I'm not sure that it is in the philosophy of Detection.. Would you have a preference? A boolean in with_nms?

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

I used the same test case than box nms et mask nms.

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • Docs updated? What were the changes:

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2024

CLA assistant check
All committers have signed the CLA.

@LinasKo
Copy link
Contributor

LinasKo commented Oct 30, 2024

Hi @YHallouard 👋

Unfortunately, this will take some time to review. To speed things up, would you have some time to do some of the following?

  1. Ideally, yes - we'd like a bool flag on with_nms.
  2. Could you please explain how soft NMS differs from the standard algorithm?
  3. Would it be simple to adapt the current algorithm by adding a boolean flag? If so, let's do that.
  4. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

Meanwhile, I'll accept this as Hacktoberfest-approved. The PR brings the improved algo to our attention, provides the code, a few tests. Hearing what tradeoffs and design considerations were made helps us a lot too! 🤝

@LinasKo LinasKo added the hacktoberfest-accepted Contribute to the notion of open-source this October! label Oct 30, 2024
@YHallouard
Copy link
Author

YHallouard commented Oct 31, 2024

Hi @YHallouard 👋

Unfortunately, this will take some time to review. To speed things up, would you have some time to do some of the following?

  1. Ideally, yes - we'd like a bool flag on with_nms.
  2. Could you please explain how soft NMS differs from the standard algorithm?
  3. Would it be simple to adapt the current algorithm by adding a boolean flag? If so, let's do that.
  4. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

Meanwhile, I'll accept this as Hacktoberfest-approved. The PR brings the improved algo to our attention, provides the code, a few tests. Hearing what tradeoffs and design considerations were made helps us a lot too! 🤝

Hi @LinasKo, And thank you very much for your review :)

this will take some time to review.

Yes I know haha, this is why I din't started the documentation, I wanted to get your feedback on implementation first.

  1. Ideally, yes - we'd like a bool flag on with_nms.

Yes, I deeply thought it was the mindset. My concerns was that I need a sigma parameter also, that will be useful only in the case where the bool flag is True. If you have any idea about that, I'll take it :D

  1. Could you please explain how soft NMS differs from the standard algorithm?

Of course, unlike NMS which removes predictions if they are below a certain IOU threshold, Soft-NMS updates the confidence score of each prediction with a continuous function of the IOU. You can then remove them later with a confidence threshold but the main idea is to preserve overlapping bounding boxes with a good confidence.

  1. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

I'll do that as soon as possible, maybe this weekend :)

Thank for your review again

@YHallouard
Copy link
Author

YHallouard commented Nov 3, 2024

Hi @LinasKo

  1. Could you please provide a Colab that shows the difference in results? Ideally, it should show improvements in the filtered detections.

Here it is :) Soft-NMS vs NMS

@LinasKo
Copy link
Contributor

LinasKo commented Nov 3, 2024

Thanks @YHallouard !

I'll have a look at the start of next week

@YHallouard
Copy link
Author

Hi @LinasKo, sorry for the message, did you had the time to look at the notebook ? :)

@LinasKo
Copy link
Contributor

LinasKo commented Dec 3, 2024

Hi @YHallouard

Unforunately not. It's on my backlog, but honestly, supervision might steal all the time I have this year.

I'd still like to give it a proper review, so lets see - maybe I can make it happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Contribute to the notion of open-source this October!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants