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

[Feature] Add Cutout transform #1022

Merged
merged 17 commits into from
Nov 30, 2021
Merged

[Feature] Add Cutout transform #1022

merged 17 commits into from
Nov 30, 2021

Conversation

lkm2835
Copy link
Contributor

@lkm2835 lkm2835 commented Nov 8, 2021

issues 1017

CutOut transform was verified in MMDetection.
Can I bring it as it is?

However, I'm worried about whether there is a copyright infringement in this PR.


paper : https://arxiv.org/abs/1708.04552
original code : CutOut, test_cutout in mmdet

@lkm2835 lkm2835 changed the title Cutout [Feature] Add Cutout transform Nov 8, 2021
@codecov
Copy link

codecov bot commented Nov 8, 2021

Codecov Report

Merging #1022 (554f1ea) into master (d665f6b) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1022      +/-   ##
==========================================
+ Coverage   89.55%   89.62%   +0.07%     
==========================================
  Files         119      119              
  Lines        6626     6673      +47     
  Branches     1034     1043       +9     
==========================================
+ Hits         5934     5981      +47     
  Misses        488      488              
  Partials      204      204              
Flag Coverage Δ
unittests 89.62% <100.00%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
mmseg/datasets/pipelines/__init__.py 100.00% <ø> (ø)
mmseg/datasets/pipelines/transforms.py 97.60% <100.00%> (+0.30%) ⬆️

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 d665f6b...554f1ea. Read the comment docs.

@Junjun2016
Copy link
Collaborator

Hi @lkm2835
Thanks for your contribution.
Could you please add more info to the PR message?
E.g. paper link, original code link, and code or info link in mmdet.
Both mmseg and mmdet belong to openmmlab, so feel free to move code or feature from mmdet to mmseg.

@Junjun2016
Copy link
Collaborator

  • Could you please add more unittests since some lines are not covered by unittests?
  • Could you please also provide some results with or without Cutout in mmseg?

@MengzhangLI
Copy link
Contributor

Hi, @lkm2835

Nice work indeed.

Please resolve comments @Junjun2016 left above. We would review your pr quickly.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 9, 2021

I checked the comment.

As soon as my personal work is over, I will resolve it.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 9, 2021

@Junjun2016

  • Could you please add more unittests since some lines are not covered by unittests?

--> I added unittests.

.

  • Could you please also provide some results with or without Cutout in mmseg?

--> 'Some results' means some my experimental results?

.

Maybe fill with ignoring label?
So we need a fill in argument for annotation? or ignore label.

--> When I experiment in mmseg, I don't fill with ignoring label.
Because, mmdet don't use ignoring label in bbox.
But, if you think Cutout need ignoring label in segmentation, I'll try to implement it.

@Junjun2016
Copy link
Collaborator

'Some results' means some of my experimental results?

Yes.

When I experiment in mmseg, I don't fill with ignoring label.
Because, mmdet don't use ignoring label in bbox.
But, if you think Cutout need ignoring label in segmentation, I'll try to implement it.

We can make a comparison of these two cases.
Could you please also provide some results? (ablation)

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 10, 2021

In my experiment(trash segmentation),

I got a 0.7236 mIoU in my valid set. upernet-swin-large was used.

I got a 0.7316 mIoU with Cutout without ignoring label.

In another experiment, result is 0.6970 --> 0.7177.

Are you sure you asked for these experimental results?
I don't have some results like ade20k, coco and cityscapes.

@Junjun2016
Copy link
Collaborator

In my experiment(trash segmentation),

I got a 0.7236 mIoU in my valid set. upernet-swin-large was used.

I got a 0.7316 mIoU with Cutout without ignoring label.

In another experiment, result is 0.6970 --> 0.7177.

Are you sure you asked for these experimental results? I don't have some results like ade20k, coco and cityscapes.

Could also provide the results with ignoring label.

@Junjun2016
Copy link
Collaborator

I don't have some results like ade20k, coco, and cityscapes.

Besides, could provide configs and results (if available) on ade20k, cityscapes both with and without ignoring label.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 10, 2021

modified ignore_index with seg_fill_in.
seg_fill_in default is None.
if seg_fill_in is not None, modify the seg_fields in results.

@Junjun2016
Copy link
Collaborator

modified ignore_index with seg_fill_in. seg_fill_in default is None. if seg_fill_in is not None, modify the seg_fields in results.

Great, thanks for your hard work.
Next, could do some ablation study.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 10, 2021

Great, thanks for your hard work.
Next, could do some ablation study.

Sorry, I'm asking because I'm not good at English.
Do I have to do some ablation study?

@Junjun2016
Copy link
Collaborator

modified ignore_index with seg_fill_in. seg_fill_in default is None. if seg_fill_in is not None, modify the seg_fields in results.

Great, thanks for your hard work. Next, could do some ablation study.

We will do some ablation study with seg_fill_in to be None and ignore label, and record the results here.
You could also provide some experimental results with different hyperparameters in CutOut.
Besides, could you please also move other nice features in mmdet to mmseg?

@RockeyCoss
Copy link
Contributor

Please merge the master branch into your branch, thank you.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 15, 2021

If possible, Could you advise me how to use Codecov for test code in my terminal? (before PR)

@Junjun2016
Copy link
Collaborator

If possible, Could you advise me how to use Codecov for test code in my terminal? (before PR)

@MengzhangLI, please.

@Junjun2016
Copy link
Collaborator

If possible, Could you advise me how to use Codecov for test code in my terminal? (before PR)

@MengzhangLI, please.

coverage run --source={package, e.g., mmdet} --branch -m pytest
coverage report -m

@MengzhangLI
Copy link
Contributor

  1. Using pytest to check unit test

pytest tests/${test_file}.py::${test_function}
pytest tests/${test_file}.py

  1. Install coverage

pip install coverage

  1. Using code below under your repo, thus you can get unittest and return its coverage rate.

coverage run --source={package, e.g., mmseg} --branch -m pytest
coverage report -m

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 15, 2021

Thank you :)

@Junjun2016
Copy link
Collaborator

  1. Using pytest to check unit test

pytest tests/${test_file}.py::${test_function}
pytest tests/${test_file}.py

  1. Install coverage

pip install coverage

  1. Using code below under your repo, thus you can get unittest and return its coverage rate.

coverage run --source={package, e.g., mmseg} --branch -m pytest
coverage report -m

pytest tests # test all files in tests folder

@Junjun2016
Copy link
Collaborator

Hi @lkm2835
We are doing some ablations now and we will merge this PR soon.
Could you please move the Mosaic to mmseg? and should adjust it to the semantic segmentation task.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 15, 2021

Hi @lkm2835 We are doing some ablations now and we will merge this PR soon. Could you please move the Mosaic to mmseg? and should adjust it to the semantic segmentation task.

I have to read code(Mosaic).
After that, I'll try PR.

@Junjun2016
Copy link
Collaborator

Hi @lkm2835 We are doing some ablations now and we will merge this PR soon. Could you please move the Mosaic to mmseg? and should adjust it to the semantic segmentation task.

I have to read code(Mosaic). After that, I'll try PR.

Thanks a lot.

@MengzhangLI
Copy link
Contributor

MengzhangLI commented Nov 15, 2021

Hi, @lkm2835

Thank you so much for your warm-hearted work.

In the future, if you have fixed the comment by reviewer, the comment would be shown as Outdated and you could click Resolve conversation bottom like the figure below:

image

The pr would be more simple ;)

Best,

@Junjun2016
Copy link
Collaborator

Hi @RockeyCoss
Please review it and give some comments if necessary.

@MengzhangLI
Copy link
Contributor

Hi, @lkm2835
Thank you for your warm-hearted help on MMSegmentation.
The table above is numerical results of us, hoping it is useful for your related project.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 19, 2021

Thank you for providing the experimental results.
What does best index mean?

@RockeyCoss
Copy link
Contributor

RockeyCoss commented Nov 30, 2021

@lkm2835 Hi, sorry for the late reply. During training, we evaluate the model and save the model for every certain number of training iterations. Best index means the index of the saved model that performs best.

@lkm2835
Copy link
Contributor Author

lkm2835 commented Nov 30, 2021

@lkm2835 Hi, sorry for the late reply. During training, we evaluate the model and save the model for every certain number of training iterations. Best index means the index of the saved model that performs best.

@RockeyCoss, Thanks for your replay.
Is there anything to need more ablation study?
Recently, I have an 1 extra GPU.

@Junjun2016
Copy link
Collaborator

@lkm2835 Hi, sorry for the late reply. During training, we evaluate the model and save the model for every certain number of training iterations. Best index means the index of the saved model that performs best.

@RockeyCoss, Thanks for your replay. Is there anything to need more ablation study? Recently, I have an 1 extra GPU.

@RockeyCoss Could report more results here.

@Junjun2016
Copy link
Collaborator

@lkm2835 Hi, sorry for the late reply. During training, we evaluate the model and save the model for every certain number of training iterations. Best index means the index of the saved model that performs best.

@RockeyCoss, Thanks for your replay. Is there anything to need more ablation study? Recently, I have an 1 extra GPU.

Not needed yet since we have already run lots of ablation studies.

@Junjun2016
Copy link
Collaborator

Hi @lkm2835
We will merge this augmentation in this version.
Thanks for your contribution again and looking forward to your PR again.

@Junjun2016 Junjun2016 merged commit 78a6ff6 into open-mmlab:master Nov 30, 2021
bowenroom pushed a commit to bowenroom/mmsegmentation that referenced this pull request Feb 25, 2022
* Fix typo in usage example

* [Feature] Add CutOut transform

* CutOut repr covered by unittests

* Cutout ignore index, test

* ignore_index -> seg_fill_in, defualt is None

* seg_fill_in is added to repr

* test is modified for seg_fill_in is None

* seg_fill_in (int), 0-255

* add seg_fill_in test

* doc string for seg_fill_in

* rename CutOut to RandomCutOut, add prob

* Add unittest when cutout is False
aravind-h-v pushed a commit to aravind-h-v/mmsegmentation that referenced this pull request Mar 27, 2023
…lab#1022)

Deprecate `init_git_repo` and `push_to_hub`, refactor `train_unconditional.py`
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.

None yet

4 participants