-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Bug fix: Handle empty instances in FCOS.
Summary: ### 🐛 One line bug info: [FCOS implementation in current master](https://github.com/facebookresearch/detectron2/blob/31ec19b3132a3ac609600802dd37b2b40a76b5c9/detectron2/modeling/meta_arch/fcos.py) is unable to handle empty instances. This bug went unnoticed because: (a) images with empty instances are usually filtered while loading COCO annotations, and (b) FCOS should not encounter empty instances with its default training hyper-parameters. However, if I switch to a more aggressive large-scale jitter (LSJ) cropping augmentation, the model may encounter an image crop without any boxes in it. This crashes training abruptly due to a size mismatch in the pairwise anchor matching matrix. ### Bug fix This PR lets FCOS handle empty instances by adding a dummy `[0, 0, 0, 0]` box labeled as background (ID = `num_classes`), similar to how it is handled in `RetinaNet` class. Training FCOS with LSJ augmentation does not crash anymore. **Additional refactor:** While I was working on a fix, I noticed some inconsistencies in variable representations and naming conventions. For example, the `pairwise_match` variable was a `(R: anchor points, M: GT boxes)` matrix — this is a transposed representation of what a `match_quality_matrix` represents in `RetinaNet` and `GeneralizedRCNN`, `(M: GT boxes, R: anchor points)`. I refactored the code to make it more uniform with D2 (11528ce) conventions and make FCOS logic flow similar to RetinaNet, given that RetinaNet was the primary baseline in FCOS paper. My changes include: - Refactoring `pairwise_match` to a `match_quality_matrix` with its representation consistent with the rest of meta architectures. Moreover, variables renaming like (`matched_boxes` –> `matched_gt_boxes`, `label` —> `gt_labels`, and `gt_index` —> `matched_indices`) make the code more consistent with the naming convention in rest of meta architectures. - Update: After ppwwyyxx , I added this explanation as a comment instead of refactoring code: ~Use a `Matcher` instead of simply doing `pairwise_match.max()`. Original code was replacing indices of unmatched anchors as `-1` and accessing GT labels/boxes by doing `gt_index.clip(0)`, which felt non-ideal.~ - Change the internal method `match_anchors` to compute and return per-instance `match_quality_matrix` (similar to using `pairwise_iou` in R-CNN). This modifies the old behavior of returning `matched_indices`. Since this method is used internally in `FCOS`, I renamed the method to `_match_anchors`. ### Any API changes? The call signature of `FCOS` is unchanged. `FCOS.match_anchors()` is now `FCOS._match_anchors()` with a different return value, but it was only used internally by `FCOS.label_anchors()`. ### Verification I verified my changes by one full training run of FCOS with ResNet-50-FPN on COCO detection, using the builtin config. The validation curves overlap very closely (orange: `master` branch, blue: with my changes).  I should have fixed the training seed in both runs... so additionally, I manually checked the equality of `pairwise_match.tranpose(0, 1)` and my `match_quality_matrix`for first 20 iterations by fixing `train.seed = 0` in config. Everything matches exactly. Pull Request resolved: #3851 Reviewed By: wat3rBro Differential Revision: D33971235 Pulled By: zhanghang1989 fbshipit-source-id: 9eca18ef79c2942588cf12ead218a4f89bc8a297
- Loading branch information
1 parent
7cad0a7
commit ef2c3ab
Showing
3 changed files
with
95 additions
and
59 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters