Skip to content

[bugfix] set default max_dets to same values as pycocotools #9105

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

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

Conversation

ritosonn
Copy link

@ritosonn ritosonn commented Oct 21, 2022

Motivation

The COCO dataset recall evaluation in mmdetection differs from the original pycocotools. This is because the mmdetection's default maxDets ([100, 500, 1000] ) are not equal to the others ([1, 10, 100]).

This change fixes issue #6884.

Modification

  • Rename proposal_nums to max_dets
    • proposal_nums seems to be used in the first stage of two-stage object detection (many proposals will appear).
    • The name max_dets is compatible to coco evaluation.
  • Change the default values to cocoEval.params.maxDets from [100, 500, 1000] to [1, 10, 100]
    • This change incorrectly sets all case to [100, 500, 1000]
    • We can fix it to set cocoEval.params.maxDets only if metric == "proposal". I didn't because you'll change this variable in other situations (such as detection of a large number of objects).

BC-breaking (Optional)

  • Default recall evaluation of COCO dataset (measured after e20c5c2 )
    • If you want to get the previous value, you can set maxDets=[100, 500, 1000] or simply see recall@100 (because recall@[100, 500, 1000] will get the same value in most experiments).

Use cases (Optional)

COCO evaluation

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  3. If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMCls.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

@CLAassistant
Copy link

CLAassistant commented Oct 21, 2022

CLA assistant check
All committers have signed the CLA.

@ZwwWayne
Copy link
Collaborator

Hi @ritosonn ,
Thanks for your kind PR. The current change will introduce BC-breaking because the proposal_nums is no longer used. To avoid that and provide a more smooth experience to the users, could you also keep the argument of proposal_nums? When evaluating AP, we can use max_det to be align with pycocotools. When evaluating proposal AR, we can still use proposal_nums.

@DiYaGo-zzh
Copy link

Hi @ritosonn ,
I have a question, I wonder if this maxDets is just in a different format than the print of pycocotools? That is, will the change of maxDets affect the calculation of its specific indicator values?And when i try to align with pycocotools and change coco.py file from your work, but when i use tools/test.py to calculate the indicator, it still prints in the previous format,as shown in the image. If i want use tools/test.py to align with pycocotools format,whether you need to modify other files?
图片

@ZwwWayne ZwwWayne changed the base branch from master to dev November 21, 2022 02:37
@ZwwWayne ZwwWayne added this to the 2.27.0 milestone Nov 21, 2022
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.

5 participants