rotation for image augmentation WIP #1955
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Some issues I notice here are: it should default to cifar10 in the script not cifar100 (next time don't change the script, run with "--dataset cifar100"). And the rotation-with-some-probability thing probably needs a flag for it, e.g. --rotation-prob. |
|
Which scripts do I need to add the new image augmentation options (i.e. rotation-degree, rotation-prob)? Only the resnet_1* or all scripts? |
|
When we add options like this we add a new script, so e.g. copy and modify
resnet_1c to resnet_1d. No need to do it for the non-resnets right now.
…On Wed, Oct 25, 2017 at 11:31 AM, Yiwen Shao ***@***.***> wrote:
Which scripts do I need to add the new image augmentation options (i.e.
rotation-degree, rotation-prob)? Only the resnet_1* or all scripts?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1955 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADJVuzEMz2ADlU5su6eaHMRJkDgzSNxQks5sv1RXgaJpZM4QBTv->
.
|
|
Do I need to also include the changes on the number of filters and epochs in the new recipe that achieves better result? For example, I can make a resnet_1d with rotation only, resnet_1e with changes on epochs and filters, and finally resnet_1f with them together. |
|
Do 1d with rotation only and 1e with more epochs and filters added to 1d.
The 1d may be useful for experimental-turnaround-time reasons.
…On Wed, Oct 25, 2017 at 3:18 PM, Yiwen Shao ***@***.***> wrote:
Do I need to also include the changes on the number of filters and epochs
in the new recipe that achieves better result? For example, I can make a
resnet_1d with rotation only, resnet_1e with changes on epochs and filters,
and finally resnet_1f with them together.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1955 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADJVu-zCCcLPO08nUWuiKYNSeeMbhSSpks5sv4logaJpZM4QBTv->
.
|
|
OK. I notice that in each recipe, there are some lines of comments on the top showing the results of that recipe. For this reason, it might take me some time to get completed results of these new recipes. (My previous experiments are mainly on cifar100) |
|
I've pushed these two recipes. But the results in top few lines are not completed. Only the result in resnet_1e for cifar100 is up-to-date. I will update other information upon getting the results. |
|
All results updated. Ready for merging. |
|
@hhadian, please review this for me.
…On Thu, Oct 26, 2017 at 10:24 PM, Yiwen Shao ***@***.***> wrote:
All results updated. Ready for merging.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1955 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADJVu914DqFZQsU9N7ghzyZCISIyobQQks5swT7jgaJpZM4QBTv->
.
|
hhadian
reviewed
Oct 27, 2017
| @@ -231,6 +241,14 @@ void PerturbImage(const ImageAugmentationConfig &config, | |||
| // [ cos(theta) -sin(theta) 0 | |||
| // sin(theta) cos(theta) 0 | |||
| // 0 0 1 ] | |||
| if (RandUniform() <= config.rotation_prob) { | |||
| BaseFloat theta = (2 * config.rotation_degree * RandUniform() - | |||
| config.rotation_degree) / 180.0 * 3.1415926; | |||
There was a problem hiding this comment.
I guess you could use Kaldi constant M_PI
| @@ -231,6 +241,14 @@ void PerturbImage(const ImageAugmentationConfig &config, | |||
| // [ cos(theta) -sin(theta) 0 | |||
| // sin(theta) cos(theta) 0 | |||
| // 0 0 1 ] | |||
| if (RandUniform() <= config.rotation_prob) { | |||
There was a problem hiding this comment.
I guess you could use Kaldi's WithProb() which makes it nicer.
Not important though.
|
Actually don't use WithProb(). I think that was an unnecessary feature and
if it could be removed and all instances replaced with RandUniform() < x,
it would be clearer to someone with general computer knowledge.
…On Thu, Oct 26, 2017 at 11:25 PM, Hossein Hadian ***@***.***> wrote:
***@***.**** commented on this pull request.
I guess it's OK for merging, but here are 2 comments to make the code
nicer:
------------------------------
In src/nnet3bin/nnet3-egs-augment-image.cc
<#1955 (comment)>:
> @@ -231,6 +241,14 @@ void PerturbImage(const ImageAugmentationConfig &config,
// [ cos(theta) -sin(theta) 0
// sin(theta) cos(theta) 0
// 0 0 1 ]
+ if (RandUniform() <= config.rotation_prob) {
+ BaseFloat theta = (2 * config.rotation_degree * RandUniform() -
+ config.rotation_degree) / 180.0 * 3.1415926;
I guess you could use Kaldi constant M_PI
------------------------------
In src/nnet3bin/nnet3-egs-augment-image.cc
<#1955 (comment)>:
> @@ -231,6 +241,14 @@ void PerturbImage(const ImageAugmentationConfig &config,
// [ cos(theta) -sin(theta) 0
// sin(theta) cos(theta) 0
// 0 0 1 ]
+ if (RandUniform() <= config.rotation_prob) {
I guess you could use Kaldi's WithProb() which makes it nicer.
Not important though.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1955 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADJVu100rZ3-KtDmU0OcWOgcPbcAIWmqks5swU07gaJpZM4QBTv->
.
|
|
Thanks. I've replaced 3.1415 with M_PI. |
YiwenShaoStephen
force-pushed
the
master
branch
from
04c679f
to
e8762d2
Compare
kronos-cm
added a commit
to kronos-cm/kaldi
that referenced
this pull request
Nov 4, 2017
* 'master' of https://github.com/kaldi-asr/kaldi: [src,scripts] nnet1-related changes: (kaldi-asr#1998) [egs] rotation for image augmentation in CIFAR example (kaldi-asr#1955) [egs] Minor fixes to the SRE16 v2 recipe (kaldi-asr#1986) [egs] Remove deprecated non-working scripts [scripts] Fixes to segment_long_utterances.sh (thanks @christophfeinauer) and train_raw_dnn.py (kaldi-asr#1993) [src] Minor fix: change to error message (kaldi-asr#1980) [egs] Add example of component-level l2-regularize for WSJ scripts [egs] Small fix to Chime4 RE data location (kaldi-asr#1966) [build] Remove download of ATLAS header files from tools/. (kaldi-asr#1974)
xiaohui-zhang
pushed a commit
to xiaohui-zhang/kaldi
that referenced
this pull request
Nov 14, 2017
mahsa7823
pushed a commit
to mahsa7823/kaldi
that referenced
this pull request
Feb 28, 2018
Skaiste
pushed a commit
to Skaiste/idlak
that referenced
this pull request
Sep 26, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.