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

Add random affine transformation #411

Merged
merged 4 commits into from
Feb 20, 2018
Merged

Conversation

vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Feb 8, 2018

Random affine transformations with tests.
For usage, see sanity_check.ipynb

format_string += ' shear={0}\n'.format(self.shear)
format_string += ' resample={0}\n'.format(self.resample)
format_string += ' fillcolor={0}\n'.format(self.fillcolor)
format_string += ')'

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 10, 2018

Any other feedbacks on the PR are highly appreciated :)

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.

This looks great, thanks!

I have a couple of minor comments, but apart from that this is very good!

format_string += ' shear={0}\n'.format(self.shear)
format_string += ' resample={0}\n'.format(self.resample)
format_string += ' fillcolor={0}\n'.format(self.fillcolor)
format_string += ')'

This comment was marked as off-topic.

@@ -734,6 +735,109 @@ def test_rotate(self):

assert np.all(np.array(result_a) == np.array(result_b))

def test_affine(self):
x = np.zeros((100, 100, 3), dtype=np.uint8)

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

if self.shear is not None:
s += ', shear={shear}'
if self.resample > 0:
s += ', resample={resample}'

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 12, 2018

@fmassa A precision on matching between transformed points. Take look at the example with scale.
Here is the input image:
input
which is then transformed with

result = F.affine(img, angle=0, translate=(0, 0), scale=2.0, shear=0.0)

result
However, manually-verified points gives :

xs = []
ys = []
true_image = np.zeros((100, 100, 3), dtype=np.uint8)
for y, x in pts:
    xs.append(int((x - cnt[0]) * s + cnt[0] + 0.5))
    ys.append(int((y - cnt[1]) * s + cnt[1] + 0.5))
    true_image[ys[-1], xs[-1], :] = (255, 255, 255)

true_image
what is correct as input points are transformed to the output and not as an inverse mapping (as in PIL).

So, I think if we keep these tests, we need to match only manually-verified set in the non-null locations of affine transformed image.
What is your opinion ?

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 14, 2018

I refactored a little the code and tests such that I compare transformation matrices and images with accepted error of max N(=3) different pixels (cause of aliasing). With such approach, tests take about 3 mins to execute because of rather bruteforce tests:

for a in range(-90, 90, 25):
    for t1 in range(-10, 10, 5):
        for s in [0.75, 0.98, 1.0, 1.1, 1.2]:
              for sh in range(-15, 15, 5):
                   _test_transformation(a=a, t=(t1, t1), s=s, sh=sh)

Thoughts @fmassa ?

@vfdev-5
Copy link
Collaborator Author

vfdev-5 commented Feb 19, 2018

@fmassa can we merge this ?

@soumith soumith merged commit f5d486c into pytorch:master Feb 20, 2018
@soumith
Copy link
Member

soumith commented Feb 20, 2018

thanks @vfdev-5

@vfdev-5 vfdev-5 deleted the random_affine branch February 20, 2018 07:01
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