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

[Fix] flip_ratio in RandomAffine pipeline is inverted #799

Merged
merged 5 commits into from
Mar 24, 2022

Conversation

wdmwhh
Copy link
Contributor

@wdmwhh wdmwhh commented Mar 17, 2022

original: flip_ratio=1. means always no flip and flip_ratio=0. means always flip
after: flip_ratio=1. means always flip and flip_ratio=0. means always no flip

original: `flip_ratio=1.` means always no flip and `flip_ratio=0.` means always flip
after:      `flip_ratio=1.` means always flip and `flip_ratio=0.` means always no flip
@mm-assistant mm-assistant bot added the size/XS label Mar 17, 2022
@wangruohui
Copy link
Member

Because flip is used as a multiplier in line 479 and 480, so -1 stands for flip and 1 stands for no flip. Thus flip should be an 'inverse' flag as the result of the comparison.

Is it correct?

@codecov
Copy link

codecov bot commented Mar 18, 2022

Codecov Report

Merging #799 (cd1328d) into master (696e3a8) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #799      +/-   ##
==========================================
+ Coverage   83.05%   83.08%   +0.02%     
==========================================
  Files         216      216              
  Lines       12192    12253      +61     
  Branches     1964     1978      +14     
==========================================
+ Hits        10126    10180      +54     
- Misses       1763     1764       +1     
- Partials      303      309       +6     
Flag Coverage Δ
unittests 83.04% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
mmedit/datasets/pipelines/augmentation.py 98.49% <100.00%> (ø)
mmedit/apis/video_interpolation_inference.py 86.40% <0.00%> (-4.83%) ⬇️
mmedit/datasets/builder.py 93.22% <0.00%> (-1.61%) ⬇️
mmedit/apis/restoration_video_inference.py 18.96% <0.00%> (-1.38%) ⬇️
...ls/components/stylegan2/generator_discriminator.py 84.93% <0.00%> (-1.21%) ⬇️
...t/models/video_interpolators/basic_interpolator.py 97.36% <0.00%> (+0.04%) ⬆️
mmedit/datasets/pipelines/blur_kernels.py 84.55% <0.00%> (+1.47%) ⬆️
mmedit/apis/train.py 30.08% <0.00%> (+12.08%) ⬆️

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 696e3a8...cd1328d. Read the comment docs.

@wdmwhh
Copy link
Contributor Author

wdmwhh commented Mar 19, 2022

Because flip is used as a multiplier in line 479 and 480, so -1 stands for flip and 1 stands for no flip. Thus flip should be an 'inverse' flag as the result of the comparison.

Is it correct?

Yes, it is.

@wangruohui wangruohui self-requested a review March 19, 2022 13:53
@wangruohui
Copy link
Member

wangruohui commented Mar 19, 2022

It seems there is another error. When flipping, the center is shifted.
I made some modifications and added some unit tests, you can have a test to verify.
I am not sure whether it is related to version of opencv

@wdmwhh, if the modification works, please let me know and I will clean the code and merge this PR.

@wangruohui wangruohui changed the title fix flip_ratio bug [Fix] flip_ratio in RandomAffine pipeline is inverted Mar 21, 2022
@wdmwhh
Copy link
Contributor Author

wdmwhh commented Mar 22, 2022

It seems there is another error. When flipping, the center is shifted. I made some modifications and added some unit tests, you can have a test to verify. I am not sure whether it is related to version of opencv

@wdmwhh, if the modification works, please let me know and I will clean the code and merge this PR.

Yes, it makes sense. Thanks for your kindness.

@wangruohui wangruohui merged commit 051a9f7 into open-mmlab:master Mar 24, 2022
biubiubiiu added a commit to biubiubiiu/derain-toolbox that referenced this pull request Apr 9, 2022
Yshuo-Li pushed a commit to Yshuo-Li/mmediting that referenced this pull request Jul 15, 2022
…#799)

* fix flip_ratio bug

original: `flip_ratio=1.` means always no flip and `flip_ratio=0.` means always flip
after:      `flip_ratio=1.` means always flip and `flip_ratio=0.` means always no flip

* add some notes to modification

* modify center, add unittests

* clean test codes

* clean test codes

Co-authored-by: wangruohui <12756472+wangruohui@users.noreply.github.com>
Yshuo-Li pushed a commit to Yshuo-Li/mmediting that referenced this pull request Jul 15, 2022
…#799)

* fix flip_ratio bug

original: `flip_ratio=1.` means always no flip and `flip_ratio=0.` means always flip
after:      `flip_ratio=1.` means always flip and `flip_ratio=0.` means always no flip

* add some notes to modification

* modify center, add unittests

* clean test codes

* clean test codes

Co-authored-by: wangruohui <12756472+wangruohui@users.noreply.github.com>
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.

2 participants