Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

remove redundant variables and lines, fix typos #904

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Johnqczhang
Copy link
Contributor

@Johnqczhang Johnqczhang commented Jun 17, 2019

  1. Remove unused variables and unused (or repeatedly imported) modules

  2. Fix typos:

    • mask -> keypoint in keypoint_head.py: Line30 and in roi_heads.py: Line45
    • gor -> for in mask_head/inference.py: Line90
    • classification -> segmentation in roi_mask_feature_extractors.py: Line19
  3. Add missing comment for argument discretization_size in mask_head/loss.py: Line22

    def project_masks_on_boxes(segmentation_masks, proposals, discretization_size):
    """
    Given segmentation masks and the bounding boxes corresponding
    to the location of the masks in the image, this function
    crops and resizes the masks in the position defined by the
    boxes. This prepares the masks for them to be fed to the
    loss computation as the targets.
    Arguments:
    segmentation_masks: an instance of SegmentationMask
    proposals: an instance of BoxList
    discretization_size: spatial resolution of masks
    """

  4. Remove incorrect comments in roi_mask_feature_extractors.py: Line23-28

    def __init__(self, cfg, in_channels):
    """
    Arguments:
    num_classes (int): number of output classes
    input_size (int): number of channels of the input once it's flattened
    representation_size (int): size of the intermediate representation
    """

  5. Add two assertions when enabling sharing features from box_head for mask/keypoint head in roi_heads.py: Line19 and Line22

    if cfg.MODEL.MASK_ON and cfg.MODEL.ROI_MASK_HEAD.SHARE_BOX_FEATURE_EXTRACTOR:
    assert cfg.MODEL.ROI_MASK_HEAD.POOLER_RESOLUTION == cfg.MODEL.ROI_BOX_HEAD.POOLER_RESOLUTION
    self.mask.feature_extractor = self.box.feature_extractor
    if cfg.MODEL.KEYPOINT_ON and cfg.MODEL.ROI_KEYPOINT_HEAD.SHARE_BOX_FEATURE_EXTRACTOR:
    assert cfg.MODEL.ROI_KEYPOINT_HEAD.POOLER_RESOLUTION == cfg.MODEL.ROI_BOX_HEAD.POOLER_RESOLUTION
    self.keypoint.feature_extractor = self.box.feature_extractor

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 17, 2019
@Johnqczhang
Copy link
Contributor Author

Hi @fmassa, can you give a code review for my PR? Thanks!

Copy link
Contributor

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay in reviewing.

It generally looks good, but there are a few things that should be reverted as they deserve more discussion

maskrcnn_benchmark/modeling/roi_heads/roi_heads.py Outdated Show resolved Hide resolved
maskrcnn_benchmark/modeling/detector/generalized_rcnn.py Outdated Show resolved Hide resolved
@Johnqczhang
Copy link
Contributor Author

Johnqczhang commented Jul 1, 2019

Hi @fmassa, I also fixed some other typos in #936 and some other changes (but not committed yet) regarding on similar issue with this PR. Should I merge it to this PR or leave it as another separate PR?

@Johnqczhang Johnqczhang mentioned this pull request Jul 1, 2019
@Johnqczhang
Copy link
Contributor Author

Johnqczhang commented Jul 18, 2019

Hi @fmassa, the review changes have been done as you requested before, can we merge this PR now?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants