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

Adjust NonMaxSuppression score_threshold in build_detection_graph for ssd networks #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cosama
Copy link

@cosama cosama commented Oct 2, 2018

It took me quite some time to figure this out, so I think people would be happy to have this automatically done for them when running the build_detection_graph function. The threshold can be adjusted by passing in an argument. With this fix, it should be possible to get the described performance without any tweaks to the models.

I used the warnings module to inform the user about if the value was changed or if it was not possible to change it. I can change that if necessary.

…n be reconfigured from build_detection_graph
@ghost
Copy link

ghost commented Oct 2, 2018

Thanks for sharing this. I agree that being able to easily set the NMS score threshold is a helpful feature. We are considering adding this, as well as some other changes to improve model support.

I share your concern that defaulting to a threshold of 0.3 may be surprising (especially if users modify their configuration directly).

Since others have shared your frustration though, I will add a note to the README to suggest modifying the score threshold directly, which will hopefully ease some pain.

@cosama
Copy link
Author

cosama commented Oct 2, 2018

I'm not too frustrated with it, just took me some time to figure out where to change that parameter. I think it would be nice for others to have that implemented right away and get what they are promised without much work.

I saw that you have added this to the Todo list in the improved_model_support branch. I can either close this for now or leave it open for later use, whatever you prefer?

@ghost
Copy link

ghost commented Oct 2, 2018

I think it's okay to leave it open for now. I will close if we merge or implement an alternative. Thanks.

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

1 participant