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

[BC-breaking] RandomErasing is now scriptable #2386

Merged
merged 2 commits into from
Jul 3, 2020

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Jul 3, 2020

Related to #2292

Description

  • RandomErasing is now scriptable

This PR changes the default behavior of get_params function, as before value=0 and now value=None which is interpreted as Gaussian random noise. If users are calling get_params explicitly this would be a BC-breaking change.

- RandomErasing is not scriptable
@codecov
Copy link

codecov bot commented Jul 3, 2020

Codecov Report

Merging #2386 into master will decrease coverage by 0.03%.
The diff coverage is 55.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2386      +/-   ##
==========================================
- Coverage   70.76%   70.72%   -0.04%     
==========================================
  Files          93       94       +1     
  Lines        7788     7870      +82     
  Branches     1209     1238      +29     
==========================================
+ Hits         5511     5566      +55     
- Misses       1913     1930      +17     
- Partials      364      374      +10     
Impacted Files Coverage Δ
torchvision/transforms/transforms.py 76.54% <54.05%> (-1.14%) ⬇️
torchvision/transforms/functional.py 78.88% <100.00%> (+0.83%) ⬆️
torchvision/io/image.py 70.96% <0.00%> (ø)
torchvision/datasets/utils.py 59.58% <0.00%> (+2.26%) ⬆️

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 39e4057...e425b76. Read the comment docs.

Copy link
Member

@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.

The PR looks great, thanks a lot @vfdev-5 !

I only have a couple of minor comments (typo and perf improvement to move the code to where it was before).
After that this is good to merge!

torchvision/transforms/transforms.py Outdated Show resolved Hide resolved
torchvision/transforms/transforms.py Outdated Show resolved Hide resolved
torchvision/transforms/transforms.py Show resolved Hide resolved
@vfdev-5 vfdev-5 changed the title RandomErasing is now scriptable [BC-breaking] RandomErasing is now scriptable Jul 3, 2020
- added additional checking of value vs img num_channels
Copy link
Member

@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.

Thanks a lot!

@fmassa fmassa merged commit 75f5b57 into pytorch:master Jul 3, 2020
@vfdev-5 vfdev-5 mentioned this pull request Jul 6, 2020
16 tasks
de-vri-es pushed a commit to fizyr-forks/torchvision that referenced this pull request Aug 4, 2020
* Related to pytorch#2292
- RandomErasing is not scriptable

* Fixed code according to review comments
- added additional checking of value vs img num_channels
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