[GitHub] piiswrong commented on a change in pull request #7345: add std_rgba to normalize options
piiswrong commented on a change in pull request #7345: add std_rgba to normalize options URL: https://github.com/apache/incubator-mxnet/pull/7345#discussion_r132511274 ## File path: src/io/iter_normalize.h ## @@ -129,33 +129,58 @@ class ImageNormalizeIter : public IIterator { if (param_.mean_r > 0.0f || param_.mean_g > 0.0f || param_.mean_b > 0.0f || param_.mean_a > 0.0f) { // subtract mean per channel - data[0] -= param_.mean_r; - if (data.shape_[0] >= 3) { -data[1] -= param_.mean_g; -data[2] -= param_.mean_b; - } - if (data.shape_[0] == 4) { -data[3] -= param_.mean_a; - } - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data * contrast + illumination) * param_.scale; - } else { -outimg_ = (data * contrast + illumination) * param_.scale; + switch (data.shape_[0]) { +case 4: { + data[3] -= param_.mean_a; +} +case 3: { + data[2] -= param_.mean_b; +} +case 2: { + data[1] -= param_.mean_g; +} +case 1: { + data[0] -= param_.mean_r; + break; +} +default: { + // possible? +} } } else if (!meanfile_ready_ || param_.mean_img.length() == 0) { // do not subtract anything - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data) * param_.scale; - } else { -outimg_ = F(data) * param_.scale; - } } else { CHECK(meanfile_ready_); - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror((data - meanimg_) * contrast + illumination) * param_.scale; - } else { -outimg_ = ((data - meanimg_) * contrast + illumination) * param_.scale; + data -= meanimg_; +} +// apply contrast and illumination jitter +data = data * contrast + illumination; +// apply std per channel +switch (data.shape_[0]) { + case 4: { +if (param_.std_a > 0.0f) data[3] /= param_.std_a; } + case 3: { +if (param_.std_b > 0.0f) data[2] /= param_.std_b; + } + case 2: { +if (param_.std_g > 0.0f) data[1] /= param_.std_g; + } + case 1: { +if (param_.std_r > 0.0f) data[0] /= param_.std_r; +break; + } + default: { +// possible? Review comment: should fail on default This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] piiswrong commented on a change in pull request #7345: add std_rgba to normalize options
piiswrong commented on a change in pull request #7345: add std_rgba to normalize options URL: https://github.com/apache/incubator-mxnet/pull/7345#discussion_r132511245 ## File path: src/io/iter_normalize.h ## @@ -129,33 +129,58 @@ class ImageNormalizeIter : public IIterator { if (param_.mean_r > 0.0f || param_.mean_g > 0.0f || param_.mean_b > 0.0f || param_.mean_a > 0.0f) { // subtract mean per channel - data[0] -= param_.mean_r; - if (data.shape_[0] >= 3) { -data[1] -= param_.mean_g; -data[2] -= param_.mean_b; - } - if (data.shape_[0] == 4) { -data[3] -= param_.mean_a; - } - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data * contrast + illumination) * param_.scale; - } else { -outimg_ = (data * contrast + illumination) * param_.scale; + switch (data.shape_[0]) { +case 4: { + data[3] -= param_.mean_a; +} +case 3: { + data[2] -= param_.mean_b; +} +case 2: { + data[1] -= param_.mean_g; +} +case 1: { + data[0] -= param_.mean_r; + break; +} +default: { + // possible? +} } } else if (!meanfile_ready_ || param_.mean_img.length() == 0) { // do not subtract anything - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data) * param_.scale; - } else { -outimg_ = F(data) * param_.scale; - } } else { CHECK(meanfile_ready_); - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror((data - meanimg_) * contrast + illumination) * param_.scale; - } else { -outimg_ = ((data - meanimg_) * contrast + illumination) * param_.scale; + data -= meanimg_; +} +// apply contrast and illumination jitter +data = data * contrast + illumination; +// apply std per channel +switch (data.shape_[0]) { + case 4: { +if (param_.std_a > 0.0f) data[3] /= param_.std_a; } + case 3: { +if (param_.std_b > 0.0f) data[2] /= param_.std_b; + } + case 2: { +if (param_.std_g > 0.0f) data[1] /= param_.std_g; + } + case 1: { +if (param_.std_r > 0.0f) data[0] /= param_.std_r; +break; + } + default: { +// possible? + } +} +// scale the values globally +data *= param_.scale; Review comment: The computation was condensed in one line because its faster that way. Are you sure contrast and illumination should happen after std? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] piiswrong commented on a change in pull request #7345: add std_rgba to normalize options
piiswrong commented on a change in pull request #7345: add std_rgba to normalize options URL: https://github.com/apache/incubator-mxnet/pull/7345#discussion_r131536401 ## File path: src/io/iter_normalize.h ## @@ -129,33 +129,58 @@ class ImageNormalizeIter : public IIterator { if (param_.mean_r > 0.0f || param_.mean_g > 0.0f || param_.mean_b > 0.0f || param_.mean_a > 0.0f) { // subtract mean per channel - data[0] -= param_.mean_r; - if (data.shape_[0] >= 3) { -data[1] -= param_.mean_g; -data[2] -= param_.mean_b; - } - if (data.shape_[0] == 4) { -data[3] -= param_.mean_a; - } - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data * contrast + illumination) * param_.scale; - } else { -outimg_ = (data * contrast + illumination) * param_.scale; + switch (data.shape_[0]) { +case 1: { + data[0] -= param_.mean_r; +} +case 2: { + data[0] -= param_.mean_g; +} +case 3: { + data[0] -= param_.mean_b; +} +case 4: { + data[0] -= param_.mean_a; + break; +} +default: { + // possible? +} } } else if (!meanfile_ready_ || param_.mean_img.length() == 0) { // do not subtract anything - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data) * param_.scale; - } else { -outimg_ = F(data) * param_.scale; - } } else { CHECK(meanfile_ready_); - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror((data - meanimg_) * contrast + illumination) * param_.scale; - } else { -outimg_ = ((data - meanimg_) * contrast + illumination) * param_.scale; + data -= meanimg_; +} +// apply contrast and illumination jitter +data = data * contrast + illumination; +// apply std per channel +switch (data.shape_[0]) { + case 1: { +if (param_.std_r > 0.0f) data[0] /= param_.std_r; } + case 2: { +if (param_.std_g > 0.0f) data[1] /= param_.std_g; + } + case 3: { +if (param_.std_b > 0.0f) data[2] /= param_.std_b; + } + case 4: { +if (param_.std_a > 0.0f) data[3] /= param_.std_a; Review comment: order needs to be reversed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] piiswrong commented on a change in pull request #7345: add std_rgba to normalize options
piiswrong commented on a change in pull request #7345: add std_rgba to normalize options URL: https://github.com/apache/incubator-mxnet/pull/7345#discussion_r131526668 ## File path: src/io/iter_normalize.h ## @@ -137,25 +137,32 @@ class ImageNormalizeIter : public IIterator { if (data.shape_[0] == 4) { data[3] -= param_.mean_a; } - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data * contrast + illumination) * param_.scale; - } else { -outimg_ = (data * contrast + illumination) * param_.scale; - } } else if (!meanfile_ready_ || param_.mean_img.length() == 0) { // do not subtract anything - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data) * param_.scale; - } else { -outimg_ = F(data) * param_.scale; - } } else { CHECK(meanfile_ready_); - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror((data - meanimg_) * contrast + illumination) * param_.scale; - } else { -outimg_ = ((data - meanimg_) * contrast + illumination) * param_.scale; - } + data -= meanimg_; +} +// apply contrast and illumination jitter +data = data * contrast + illumination; +// apply std +if (param_.std_r > 0.0f) { + data[0] /= param_.std_r; +} +if (data.shape_[0] >= 3 && param_.std_g > 0.0f) { Review comment: >=2? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] piiswrong commented on a change in pull request #7345: add std_rgba to normalize options
piiswrong commented on a change in pull request #7345: add std_rgba to normalize options URL: https://github.com/apache/incubator-mxnet/pull/7345#discussion_r131526676 ## File path: src/io/iter_normalize.h ## @@ -137,25 +137,32 @@ class ImageNormalizeIter : public IIterator { if (data.shape_[0] == 4) { data[3] -= param_.mean_a; } - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data * contrast + illumination) * param_.scale; - } else { -outimg_ = (data * contrast + illumination) * param_.scale; - } } else if (!meanfile_ready_ || param_.mean_img.length() == 0) { // do not subtract anything - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror(data) * param_.scale; - } else { -outimg_ = F(data) * param_.scale; - } } else { CHECK(meanfile_ready_); - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { -outimg_ = mirror((data - meanimg_) * contrast + illumination) * param_.scale; - } else { -outimg_ = ((data - meanimg_) * contrast + illumination) * param_.scale; - } + data -= meanimg_; +} +// apply contrast and illumination jitter +data = data * contrast + illumination; +// apply std +if (param_.std_r > 0.0f) { + data[0] /= param_.std_r; +} +if (data.shape_[0] >= 3 && param_.std_g > 0.0f) { Review comment: a switch without break would look better here This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] piiswrong commented on a change in pull request #7345: add std_rgba to normalize options
piiswrong commented on a change in pull request #7345: add std_rgba to normalize options URL: https://github.com/apache/incubator-mxnet/pull/7345#discussion_r131526450 ## File path: src/io/iter_normalize.h ## @@ -137,25 +137,32 @@ class ImageNormalizeIter : public IIterator { if (data.shape_[0] == 4) { data[3] -= param_.mean_a; } - if ((param_.rand_mirror && coin_flip(rnd_)) || param_.mirror) { Review comment: why remove these? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services