Skip to content

Conversation

@hachreak
Copy link
Contributor

Signed-off-by: Leonardo Rossi leonardo.rossi@unipr.it

Motivation

Original code for the paper: Recursively Refined R-CNN: Instance Segmentation with Self-RoI Rebalancing

Modification

R3RoIHead is a more generic version of HybridTaskCascadeRoIHead, in the sense that permits to specify which bbox/mask head use at each step.

BC-breaking (Optional)

It should not break anything.

@CLAassistant
Copy link

CLAassistant commented Oct 28, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Oct 28, 2021

Codecov Report

Merging #6399 (755e03c) into master (6cf9aa1) will decrease coverage by 0.06%.
The diff coverage is 27.29%.

❗ Current head 755e03c differs from pull request most recent head 4c2e563. Consider uploading reports for the commit 4c2e563 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6399      +/-   ##
==========================================
- Coverage   62.03%   61.97%   -0.07%     
==========================================
  Files         319      321       +2     
  Lines       25314    25386      +72     
  Branches     4189     4211      +22     
==========================================
+ Hits        15703    15732      +29     
- Misses       8783     8824      +41     
- Partials      828      830       +2     
Flag Coverage Δ
unittests 61.94% <27.29%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmdet/models/roi_heads/r3_roi_head.py 25.35% <25.35%> (ø)
mmdet/models/roi_heads/mask_heads/r3_mask_head.py 27.58% <27.58%> (ø)
mmdet/models/roi_heads/cascade_roi_head.py 36.72% <66.66%> (-0.21%) ⬇️
mmdet/models/roi_heads/__init__.py 100.00% <100.00%> (ø)
mmdet/models/roi_heads/htc_roi_head.py 100.00% <100.00%> (+71.38%) ⬆️
mmdet/models/roi_heads/mask_heads/__init__.py 100.00% <100.00%> (ø)
mmdet/models/backbones/pvt.py 83.59% <0.00%> (-0.45%) ⬇️
mmdet/apis/__init__.py 100.00% <0.00%> (ø)
mmdet/models/backbones/swin.py 81.63% <0.00%> (+0.06%) ⬆️
mmdet/apis/train.py 16.00% <0.00%> (+0.09%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cf9aa1...4c2e563. Read the comment docs.

@ZwwWayne ZwwWayne requested a review from hhaAndroid November 2, 2021 13:17
@ZwwWayne
Copy link
Collaborator

ZwwWayne commented Nov 2, 2021

Hi @hachreak ,
Thanks for your kind contribution. Would you like to fix the lint issue first? You can click the buttons in this section to see the details.
image

@hachreak
Copy link
Contributor Author

hachreak commented Nov 3, 2021

Would you like to fix the lint issue first?

Hi @ZwwWayne , thanks for your comment!
I rebased and I hope to have fixed the issues now (I run pre-commit run --all-files). 😄

@hhaAndroid
Copy link
Member

@hachreak Please add the unit test, because the code coverage drops more.

Signed-off-by: Leonardo Rossi <leonardo.rossi@unipr.it>
@@ -0,0 +1,14 @@
# Recursively Refined R-CNN: Instance Segmentation with Self-RoI Rebalancing
Copy link
Member

Choose a reason for hiding this comment

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

Like other algorithms, the precision table needs to be added.

@hhaAndroid
Copy link
Member

@hachreak Please confirm whether the Cascade Mask RCNN has BC breaking. At the same time, you can simply run an experiment to see if there are any problems. If you need other help, you can contact me.

@hachreak
Copy link
Contributor Author

Hi @hhaAndroid
I think it should not break anything for Cascade R-CNN and neither for HTC.
But I did't consider Cascade Mask RCNN.
Where is the source code for that model to check?

Actually, to run experiments for me now could be a problem because I don't have enough gpus to start some training. 😞

@hachreak
Copy link
Contributor Author

Hi @hhaAndroid any news for the PR? 😄
How could I help to speed up the integration?

@ZwwWayne ZwwWayne changed the base branch from master to dev February 16, 2022 01:45
@ZwwWayne
Copy link
Collaborator

  1. Conflicts should be resolved.
  2. htc_roi_head should not be deleted.
  3. cascade_roi_head.py should not be modified. We suggest restricting the modification inside the r3_roi_head.py You can override these functions in cascade_roi_head.py

@hachreak
Copy link
Contributor Author

Hi @ZwwWayne
thanks to answering 😄

  1. of course! I'll do it
  2. actually I'm not deleting but generalize it, because R3 RoI Head is a generalization of HTC RoI Head, where I can select the BBox Head to use each time. I could implement a R3 RoI Head separately but it means doubling 90% of the code. Do you think I should reimplement anyway everything? Maybe I can find another way to do it. let me think about it 😄
  3. I needed to make small changes (but without affecting the Cascade behaviour) to permit me to implement the loop. I mean, I needed to be able to select the BBox Head I want during each step of the for loop (the for loop which implement the cascade, and in my case the loop).
    Basically, for Cascade it is mandatory to select the next one, in my case I configure it.

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.

4 participants