search

open-mmlab / mmdetection

OpenMMLab Detection Toolbox and Benchmark
https://mmdetection.readthedocs.io
Apache License 2.0
29.02k stars 9.36k forks source link

Suspicious implementation of multi-class version _focal_loss_cost #10880

Open tjyuyao opened 1 year ago

tjyuyao commented 1 year ago

https://github.com/open-mmlab/mmdetection/blob/f78af7785ada87f1ced75a2313746e4ba3149760/mmdet/models/task_modules/assigners/match_cost.py#L252C1-L269C38

When the input is not binary, why in line 262 sigmoid is used instead of softmax? In the sigmoid case, the score is not normalized among classes IMO. Further, after the line 268, only the score of the correct class is selected to for training, and why minus neg_cost (which I view as a repeatition of positive pos_cost, according to the analysis of monoticity)?. This implementation is not correct IMO. Please also refer to this link https://zhuanlan.zhihu.com/p/308290543 .

Please let me know if I missed something. Thank you very much!

hhaAndroid commented 1 year ago

@tjyuyao The reason is that focal loss does not actually support softmax mode.

tjyuyao commented 1 year ago

@hhaAndroid I read the focal loss paper which says it should be naturally extended to multi-class version, and the zhihu link seems to provide a correct implementation, which is very similar to the implementation here if sigmoid is changed to softmax. Without softmax, the score of the negative classes can not be trained to decrease which I think is quite different from the idea of cross entropy.

I try to search your answer "The reason is that focal loss does not actually support softmax mode." and fail to find any references. Would you kindly explain why not the case?

tjyuyao commented 1 year ago

@jshilong May I have your suggestions on this issue? I have read #4559 and it would be very nice to have opinions from someone familiar with these lines of code. Thanks in advance!

Wolfybox commented 5 months ago

@hhaAndroid @tjyuyao I had the same question. Is it solved now?