-
Notifications
You must be signed in to change notification settings - Fork 26.2k
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 top_k
argument to post-process of conditional/deformable-DETR
#22787
Add top_k
argument to post-process of conditional/deformable-DETR
#22787
Conversation
The documentation is not available anymore as the PR was closed or merged. |
@CreatlV Thanks for opening this PR! Having the processor be compatible with the model is definitely something we want and updating the code to make it less brittle is a great initiative. At the moment, with @NielsRogge For the number of boxes returned, the default |
@amyeroberts it was done to reflect the original code, as linked in his message. The probabilities get reshaped to Making this more general makes sense. Note that we typically filter them based on a certain threshold; we first filter the 300 queries to get the top 100 recognized objects, and then set a threshold like 0.9 to only get the predictions with a score higher than 0.9. Both the |
I added |
top_k
argument to post-process of conditional/deformable-DETR
src/transformers/models/conditional_detr/image_processing_conditional_detr.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this and iterating!
Just one small comment on reshaping probs
. After that we're good to merge :)
src/transformers/models/conditional_detr/image_processing_conditional_detr.py
Outdated
Show resolved
Hide resolved
src/transformers/models/deformable_detr/image_processing_deformable_detr.py
Outdated
Show resolved
Hide resolved
87420b1
to
b9ef81b
Compare
Thanks again for adding this improvement and iterating! 🎉 |
…uggingface#22787) * update min k_value of conditional detr post-processing * feat: add top_k arg to post processing of deformable and conditional detr * refactor: revert changes to deprecated methods * refactor: move prob reshape to improve code clarity and reduce repetition
…uggingface#22787) * update min k_value of conditional detr post-processing * feat: add top_k arg to post processing of deformable and conditional detr * refactor: revert changes to deprecated methods * refactor: move prob reshape to improve code clarity and reduce repetition
…uggingface#22787) * update min k_value of conditional detr post-processing * feat: add top_k arg to post processing of deformable and conditional detr * refactor: revert changes to deprecated methods * refactor: move prob reshape to improve code clarity and reduce repetition
What does this PR do?
The current post-processing for object detection methods of deformable and conditional DETR assumes the number of classes * the number of object queries > 100. This reflects the original code in the deformable-DETR repository. However, this limits the flexibility of training on datasets with fewer classes/object queries. This PR suggests updating the post process for object detection code not to break if n_classes * n_object_queries < 100.
This PR suggests adding
top_k
argument to post-process functions of conditional/deformable-DETR with the default value of the previously hard-coded value.Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@amyeroberts, as you added these models do you think this approach is a reasonable addition?