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

Update RCNN-family docstrings #8228

Closed
rizavelioglu opened this issue Jan 22, 2024 · 1 comment · Fixed by #8231
Closed

Update RCNN-family docstrings #8228

rizavelioglu opened this issue Jan 22, 2024 · 1 comment · Fixed by #8231

Comments

@rizavelioglu
Copy link
Contributor

This issue is about docstrings in MaskRCNN, FasterRCNN, RegionProposalNetwork classes. Since it's not a bug, or a documentation (../index.html) issue, I opened a blank issue.

Issue

Possible fix:

  • Update both docstrings in mask_rcnn.py and faster_rcnn.py for the argument:
"""
rpn_score_thresh (float): only return proposals with an objectness score greater than rpn_score_thresh
"""
  • And in rpn.py, add the following argument inside the docstring:
"""
score_thresh (float): only return proposals with an objectness score greater than score_thresh
"""

Explanation:

Both arguments (rpn_score_thresh & box_score_thresh) are first passed to FasterRCNN:

super().__init__(
backbone,
num_classes,
# transform parameters
min_size,
max_size,
image_mean,
image_std,
# RPN-specific parameters
rpn_anchor_generator,
rpn_head,
rpn_pre_nms_top_n_train,
rpn_pre_nms_top_n_test,
rpn_post_nms_top_n_train,
rpn_post_nms_top_n_test,
rpn_nms_thresh,
rpn_fg_iou_thresh,
rpn_bg_iou_thresh,
rpn_batch_size_per_image,
rpn_positive_fraction,
rpn_score_thresh,
# Box parameters
box_roi_pool,
box_head,
box_predictor,
box_score_thresh,

where rpn_score_thresh is passed to RegionProposalNetwork:
rpn = RegionProposalNetwork(
rpn_anchor_generator,
rpn_head,
rpn_fg_iou_thresh,
rpn_bg_iou_thresh,
rpn_batch_size_per_image,
rpn_positive_fraction,
rpn_pre_nms_top_n,
rpn_post_nms_top_n,
rpn_nms_thresh,
score_thresh=rpn_score_thresh,

and box_score_thresh is passed to RoIHeads:
roi_heads = RoIHeads(
# Box
box_roi_pool,
box_head,
box_predictor,
box_fg_iou_thresh,
box_bg_iou_thresh,
box_batch_size_per_image,
box_positive_fraction,
bbox_reg_weights,
box_score_thresh,

In RegionProposalNetwork (here the score_thresh argument is missing in the docstring), rpn_score_thresh is used inside filter_proposals function to filter proposals based on the objectness score:

objectness_prob = torch.sigmoid(objectness)
final_boxes = []
final_scores = []
for boxes, scores, lvl, img_shape in zip(proposals, objectness_prob, levels, image_shapes):
boxes = box_ops.clip_boxes_to_image(boxes, img_shape)
# remove small boxes
keep = box_ops.remove_small_boxes(boxes, self.min_size)
boxes, scores, lvl = boxes[keep], scores[keep], lvl[keep]
# remove low scoring boxes
# use >= for Backwards compatibility
keep = torch.where(scores >= self.score_thresh)[0]

and the filter_proposals function is used during both training and inference:
boxes, scores = self.filter_proposals(proposals, objectness, images.image_sizes, num_anchors_per_level)

Hence, the docstring should change from: "during inference, only return proposals with a classification score greater than rpn_score_thresh" to: "only return proposals with an objectness score greater than rpn_score_thresh"


In RoIHeads, box_score_thresh is used inside postprocess_detections:

# remove low scoring boxes
inds = torch.where(scores > self.score_thresh)[0]

and the postprocess_detections is only executed during inference:
if self.training:
if labels is None:
raise ValueError("labels cannot be None")
if regression_targets is None:
raise ValueError("regression_targets cannot be None")
loss_classifier, loss_box_reg = fastrcnn_loss(class_logits, box_regression, labels, regression_targets)
losses = {"loss_classifier": loss_classifier, "loss_box_reg": loss_box_reg}
else:
boxes, scores, labels = self.postprocess_detections(class_logits, box_regression, proposals, image_shapes)

Hence, the docstring is correct for box_score_thresh.


In case my observations are correct, I would be glad to submit a PR.

@NicolasHug
Copy link
Member

NicolasHug commented Jan 24, 2024

Thanks a lot for the detailed report @rizavelioglu, I think you're 100% correct! We'd love to review your PR. On top of MaskRCNN and FasterRCNN, it looks like this is affecting MaskRCNN (edit: KeypointRCNN!) as well?

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 a pull request may close this issue.

2 participants