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

Score calculation as a product of the bbox and maskiou probability #317

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

Conversation

pieterblok
Copy link

@dbolya Hi Daniel, I found something strange in the postprocess function (in layers/output_utils.py). I'm using the yolact_plus_base_config settings.

In the first section of the function there is a filtering on the score_threshold (line 43). Later in the function there is a recalculation of the scores using the maskiou_p (line 86).

Shouldn't it be better that the filtering on the score_threshold is done after this calculation (applying the filer on the product of the bbox and maskiou probability)?

@dbolya
Copy link
Owner

dbolya commented Feb 5, 2020

Good catch, but that threshold was only meant to be a preliminary one to reduce processing time. The real threshold happens in prep_display, which in the YOLACT++ case would threshold again using the reweighted score. Since it's not possible that the score increases from rescoring, this shouldn't be an issue.

Though, there is a question of how all-in-one the postprocess function should be. I intended score_threshold to be there just for extra speed and for the user to threshold the score themselves, but looking back that makes no sense huh. I guess then we should fix this after all?

Also, I think your fix is missing the application of keep to the masks, since they get generated before the score reweighting (since they're necessary for that). You probably can just change

masks   = dets['mask']

on L104 to

masks   = masks[keep]

but I'd need to test.
(alternatively add the masks back to the dets dict like you did with the scores).

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.

2 participants