Making RandomErasing Out of place by default and improvising tests for the same #1060
Conversation
surgan12
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1060 +/- ##
==========================================
- Coverage 63.95% 63.91% -0.04%
==========================================
Files 66 66
Lines 5279 5282 +3
Branches 793 794 +1
==========================================
Hits 3376 3376
- Misses 1672 1674 +2
- Partials 231 232 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for the PR, having more tests is always better!
I just realized that there is a problem with the current RandomErasing: it performs the operation in-place. I think that we might want to do it out-of-place, because in-place operations are generally not expected.
Given that RandomErasing has not been in any release yet, I think it's ok to make it out-of-place right away.
Can you update the PR fixing it, and modifying the tests for the new behavior?
test/test_transforms.py
Outdated
| # Check if the unerased region is preserved | ||
| img_output = F.erase(img, i, j, h, w, v) | ||
| erased_region = torch.zeros([3, 60, 60], dtype=torch.float32) | ||
| erased_region[:, i:i + h, j:j + w] = v |
There was a problem hiding this comment.
Looks like erased_region is all zeros, as v is zero?
test/test_transforms.py
Outdated
| erased_region = torch.zeros([3, 60, 60], dtype=torch.float32) | ||
| erased_region[:, i:i + h, j:j + w] = v | ||
|
|
||
| assert torch.equal(img - img_output, erased_region) |
There was a problem hiding this comment.
if erased_region is all zeros, this means that img is equal to img_output.
There was a problem hiding this comment.
Yep, will change the value in the tests.
|
@fmassa sure ! will make it out of place. |
surgan12
changed the title
|
@fmassa ready for a review !! |
This PR includes:
cc: @fmassa