Skip to content
This repository was archived by the owner on Jan 7, 2025. It is now read-only.

Support for train data augmentation - #330

Closed
groar wants to merge 3 commits into
NVIDIA:masterfrom
Deepomatic:dataaugment
Closed

Support for train data augmentation#330
groar wants to merge 3 commits into
NVIDIA:masterfrom
Deepomatic:dataaugment

Conversation

@groar

@groar groar commented Sep 25, 2015

Copy link
Copy Markdown

Added data augmentation to classification dataset creation. Only the train set is augmented at db creation with new images obtained by a combination of the following transformations :

  • Rotation
  • Translation
  • Contrast modulation
  • Hue modulation

There are a few parameters for each transformation, including the probability for each image of being augmented.

Dataset augmentation

For now, only added tests to tools/create_db, but will add a few more regarding task creation and utils/image. Edit: done.

Translating is quite slow, so I should check if the way I do it can be improved.

TODO before merge

  • Fix hue modulation and translation
  • Add the possibility of setting the number of images generated out of each training sample
  • Fix the UI

@lukeyeager

Copy link
Copy Markdown
Member

Wow this is awesome, great work! My little smoketests all look good so far, I'll try to dig into the code some more today and see if I have any feedback for you.

Now is when I really wish we had #21 so I could see this stuff in action ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just do here what you did below at line 171. I think it's better to keep the number of changed lines small than preserve indentation. I sort of regret hand-aligning indentation in general.

@lukeyeager

Copy link
Copy Markdown
Member

Really great stuff. I've mentioned a few nitpicks above (and the licensing thing - hopefully you can find a good solution for that). Otherwise, this LGTM!

@jmancewicz, any thoughts?

@groar

groar commented Sep 26, 2015

Copy link
Copy Markdown
Author

Wow, the SO license is a good point I should have checked ! I replaced those functions with matplotlib ones. It seems they are already implemented in Pillow (see python-pillow/Pillow#816) but I couldn't figure out for sure, since .convert('HSV') is unavailable.

I also ignored hue modulation in the grayscale case since it does not make sense (we should do something better, like disabling the checkbox in the "new dataset" view).

I think I have corrected all the things you've mentioned. Tell me if you see something else.

@groar groar mentioned this pull request Sep 28, 2015
@lukeyeager

Copy link
Copy Markdown
Member

@groar, Awesome, thanks for the changes! Can you squash this down and remove the extra commits?

@gheinrich, @jmancewicz - I'd feel better if one of y'all took a look at this too before merging. I don't have time to test this exhaustively, but it looks pretty good to me.

minor fixes

minor fixes

added some tests

Minor fixes, removed the licensed functions

fixed create_db tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think trailing spaces should be avoided if possible (especially here since there is no functional change to this line)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there's a lot of this. I use this plugin (shameless plug), which makes the whitespace issues super obvious.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My text editor (visual slick) has an option to automatically remove trailing spaces.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I finally installed a sublime plugin to highlight those trailing spaces! I should be able not to add any new trailing space at least.

@gheinrich

Copy link
Copy Markdown
Contributor

[EDITED]

I have a question about the terminology here: in my understanding 'data augmentation' is a method to artificially create many unseen training examples by performing transformations on existing samples from the dataset. Am I understanding correctly that this PR does not actually increase the number of training samples? In this case is it technically correct to call this 'data augmentation'? I am asking candidly as I am not sure my understanding of the term is correct.

Would it be useful to add an option to let the user specify a measure of how many images may be generated out of each training sample in the original dataset? (as opposed to just the probability that each transformation is applied)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a test with IMAGE_CHANNELS=3 so you can test hue augmentation?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum yeah I don't know why I added those lines, they are testing nothing. The real tests are done in TestAugmentedDataset (which is only testing rotation right now). I will test the different augmentations yes.

@groar

groar commented Sep 29, 2015

Copy link
Copy Markdown
Author

Would it be useful to add an option to let the user specify a measure of how many images may be generated out of each training sample in the original dataset? (as opposed to just the probability that each transformation is applied)

@gheinrich Yes this is indeed important. We should have both a given probability and the possibility of getting several images out of an original training sample for a given transformation. I will do that as soon as I get some time.

I also need to fix the transformations as they are not correct. The rotation is intended to produce random noise instead of a black background, and it produces very bad quality images. And the hue transformation should of course not modulate the hue of the random noise that's been added by the resize!

@lukeyeager

Copy link
Copy Markdown
Member

@groar, it sounds like you have some more things to do before you feel like this is ready to merge.

Can you add a TODO list to the original post (like #209, #226, #268) so we can see the progress?

@jmancewicz

Copy link
Copy Markdown
Contributor

First off, this is awesome... but I have a few initial thoughts

A little thing, just about the code. Can you try to not leave trailing whitespaces. Future changes, to whitespace can cause merge and ownership issues. It's a good practice to strip them before commit.

I think it makes sense to move the transformation matrix operations together. I suggest moving 'Perform translation on images' beneath 'Perform rotations on images'. Also, and this is being picky, but translation is singular and rotations is plural. Actually, I'd skip changing that and make the phrases more succinct. Perhaps...

augment

or

augment2

That might be too vague, but I doubt it. The tooltips make everything clear.

Generally, field labels are capitalized [Image size](some are title style [Resize Transformation], and maybe should be changed), but with the exception of the 'dx' and 'dy', I recommend capitalization of these labels.

I'm conflicted about the unit of dx and dy. I'm fine with it being in normalized image units, but wonder if % would make it clearer what's going to happen to the images.

I'll dig into the code now.

@jmancewicz

Copy link
Copy Markdown
Contributor

Would it be better to not use clamp to edge wrap mode? Is it better to use a solid color like the rotate does?

translate

@jmancewicz

Copy link
Copy Markdown
Contributor

Also, the hue modulation does not appear to be modulation. It looks like it's setting the hue to a constant for each new image. That might be fine, but I expected it to rotate the hue.

by changing the code from

hsv[:,:, 0] = angle / 360.0

to
hsv[:,:, 0] += angle / 360.0
hsv[:,:, 0] %= 1
or if you wanted to extend the range from [0,360] to [-360,360], then
hsv[:,:, 0] += angle / 360.0 + 1.0
hsv[:,:, 0] %= 1
should do. That's all assuming that you meant to shift the hue. If the intent was to set the hue, then maybe we shouldn't call it modulation. I'm not familiar with how hue affects training. It almost seems like a rgb mult would do better for simulating varying lighting conditions.

@jmancewicz

Copy link
Copy Markdown
Contributor

Yes, the translation is very slow. I wonder if it is from the clamp to edge wrap mode. I expect this could be made much faster using OpenGL.

@groar

groar commented Sep 30, 2015

Copy link
Copy Markdown
Author

@jmancewicz

  • you're right about the hue, it should be a shift, not just setting it. It's not completely clear though what transformations work best in general as it heavily depends on the task (I don't know of any comprehensive study on data jittering, even on specific tasks. If you know any let me know I'm interested !). I personally had significant improvements on classification and regression tasks by modulating the hue, as well as by tinting images (by simply setting the hue).
    In fact, I think it would be even better to perform general HSV modulation (so it can be used to produce several useful transformations, see for example: http://papers.nips.cc/paper/5548-discriminative-unsupervised-feature-learning-with-convolutional-neural-networks.pdf).
  • Concerning the translation, you're right, it should be random noise or solid color.
  • I also agree with changing the labels. I'm not sure about the unit of dx and dy though.

@lukeyeager yes that's a good idea, I'll do that.

@lukeyeager

Copy link
Copy Markdown
Member

Closing as abandoned.

@lukeyeager lukeyeager closed this Jan 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants