-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
I'm ok with this change, but there are a few typos in the names of the options which I'd rather have fixed before.
Also, did you train any model adding augmentation? If yes, did it change anything on the expected performance?
# Image ColorJitter | ||
_C.INPUT.BRIGHTNETSS = 0.0 | ||
_C.INPUT.CONTRAST = 0.0 | ||
_C.INPUT.SITURATION = 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: it should be SATURATION
@@ -54,6 +54,12 @@ | |||
# Convert image to BGR format (for Caffe2 models), in range 0-255 | |||
_C.INPUT.TO_BGR255 = True | |||
|
|||
# Image ColorJitter | |||
_C.INPUT.BRIGHTNETSS = 0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: BRIGHTNESS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
I have correct the spelling, please tell me if there needs further modifications.
As for the performance, I actually observed no obvious improvements on our own datasets with brightness=0.4, contrast=0.4, saturation=0.4, hue=0.3
. The random seed is not manually assigned and the performance is on par with the non-color-aug version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, do you think we should add a random_seed
field in the configuration file for convenience of assigning the seed for random operations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a seed would be useful I think, even though not everything would be deterministic.
Hi, Is there further modification requirements, or is this pull request not really necessary ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
That was my suspicion, that it would not meaningfully improve performance, but why not have it for completeness!
Thanks for the PR!
* add color jitter augmentation * fix spelling (cherry picked from commit 862347d)
* add color jitter augmentation * fix spelling
Inspired by this paper, maybe the augmentation of adjusting color, brightness, contrast and saturation will make the model get trained more stably.