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

plot 5th, 50th and 95th percentile of value and derivate instead of p… #1472

Merged
merged 1 commit into from Apr 15, 2017

Conversation

LvHang
Copy link
Contributor

@LvHang LvHang commented Mar 3, 2017

@danpovey
Hi Dan,
1.I print the percentiles instead of the mean+stddev in the plots.
2.I have already tested it using 1-4 exp-dirs respectively. I'm sure the lenged will not cover the figures. The result in /export/a11/hlyu/kaldi/egs/swbd/s5c/report{1..4} respectively. You can check them.
3.In code, I still reserved the mean/stddev values and just commented the code about ploting them. Because I think it will be easy for others use it if they want.

Best wishes,
Hang

@danpovey
Copy link
Contributor

danpovey commented Mar 3, 2017

thanks.
best to delete the old code, people can get it from the git history if they want.

@LvHang
Copy link
Contributor Author

LvHang commented Mar 4, 2017

Ok, another question about the format of nonlinearity log file in report directory.
The original format is 'Iteration\tValueMean\tValueStddev\tDerivMean\tDerivStddev\n'
You hope the new format is 'Iteration\tValue_5th\tValue_50th\tValue_95th\tDeriv_5th\tDeriv_50th\tDeriv_95th\n"'
or
'Iteration\tValueMean\tValueStddev\tDerivMean\tDerivStddev\tValue_5th\tValue_50th\tValue_95th\tDeriv_5th\tDeriv_50th\tDeriv_95th\n"'

@danpovey
Copy link
Contributor

danpovey commented Mar 4, 2017 via email

@LvHang
Copy link
Contributor Author

LvHang commented Mar 4, 2017

Hi Dan,
I have already deleted the code about ploting mean+-stddev.
And I choose the format "Iteration ValueMean ValueStddev DerivMean DerivStddev Value_5th Value_50th Value_95th Deriv_5th Deriv_50th Deriv_95th" for log files.
I test them, the results are stored in /export/a11/hlyu/kaldi/egs/swbd/s5c/report{1..4} respectively.
Please check it. (I will send a example with 4 curves by email, I think it's easy for you to see.)

Best wishes,
Hang

@danpovey
Copy link
Contributor

@LvHang, is this ready?

@LvHang
Copy link
Contributor Author

LvHang commented Mar 25, 2017

@danpovey
Hi Dan,
I update the code. Although the appearance looks ok, the code look duty. Let me consider how to change.
Bests,
Hang

@LvHang
Copy link
Contributor Author

LvHang commented Mar 30, 2017

@danpovey
Hi Dan,
Could you give me some suggestions about how to modifying the codes? Now I think the figure(output of the codes) could satisfy your requirement, but I think the code looks not satisfying.
Hang

@vijayaditya
Copy link
Contributor

vijayaditya commented Mar 30, 2017 via email

@LvHang
Copy link
Contributor Author

LvHang commented Mar 30, 2017

@vijayaditya
Thank you for your suggestion. Let me fix it.

@LvHang
Copy link
Contributor Author

LvHang commented Apr 13, 2017

@danpovey
Hi Dan,
As vija's suggestion--reduce the size of your functions. I modified the style of the python scripts. Move the shared codes to a standalone function as much as possible.
At the same time, I do some tests on lstm/non-lstm model. You can check the final result in /export/a11/hlyu/kaldi/egs/swbd/s5c/reprot[34][_non_lstm] directories.
Thanks a lot.
Hang

@danpovey
Copy link
Contributor

Thanks!
I skimmed it very quickly and didn't see a problem.. if you say it's working I am inclined to believe it. @vijayaditya, unless you have any immediate objection I'd merge this.

deriv_mean, deriv_stddev]

if component_type == 'LstmNonlinearity':
parse_regex_lstmp = re.compile(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this function is still big. I recommend moving this code to a new function.

figfile_name = '{dir}/nonlinstats_{comp_name}.pdf'.format(
dir=output_dir, comp_name=comp_name)
fig.savefig(figfile_name, bbox_extra_artists=(lgd,),
if stats_per_dir[exp_dir][component_name]['type'] == 'LstmNonlinearity':
Copy link
Contributor

Choose a reason for hiding this comment

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

This code could benefit from few comments. Sorry that I didn't add any even in the original code. Could you please do it in this commit ?

iteration = int(groups[0])
component_name = groups[1]
component_type = groups[2]
value_percentiles = groups[3+gate_index*6]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think you could reduce this verbosity using a for loop and a dictionary ?

@@ -27,6 +28,51 @@ def __init__(self, message = None):
"There was an error while trying to parse the logs."
" Details : \n{0}\n".format(message))

# This function is used to fill stats_per_component_per_iter table with the
# results of regular expression.
def fill_stats_table_with_regex_result(groups, gate_index, stats_table):
Copy link
Contributor

Choose a reason for hiding this comment

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

better rename the function to specify that these are non-linearity stats.

…loting mean+-stddev

delete the code about ploting mean+-stddev

modify the style of legend

plot all the lstm gates

modify the python-script style

According to vija's suggestion, fix the code
@LvHang
Copy link
Contributor Author

LvHang commented Apr 13, 2017

@vijayaditya
Thank you for your suggestions. Please check it.
(1) I think the main point of "parse_progress_logs_for_nonlinearity_stats" function looks still big is the regular expression is too long. So I move it out of the function.
(2) I summarized the steps of "generate_nonlin_stats_plots" function and add few comments at the beginning of the function.
(3) I rename the "fill_tats_table_with_regex_result" to "fill_nonlin_stats_table_with_regex_result" so that it could show the meaning of nonlinearity statistics.
(4) As I want to save the semantic of the variables in "fill_nonlin_stats_table_with_regex_result", so I didn't convert it to a loop and dict.

Best wishes,
Hang

@vijayaditya
Copy link
Contributor

@danpovey LGTM

@danpovey
Copy link
Contributor

Thanks! Merging.

@danpovey danpovey merged commit 9e06caa into kaldi-asr:master Apr 15, 2017
danpovey added a commit that referenced this pull request Apr 21, 2017
* [srcipts] steps/nnet3/report/generate_plots.py: plot 5,50,95th percentile of value and derivative instead of mean+-stddev (#1472)

* Update travis.yml so PRs to kaldi_52 are built

* Setting up basic structure for CIFAR directory.

* [src] Some code changes/additions to support image recognition applications of nnet3

* Adding results for using batchnorm components instead of renorm

* Some partial work on CIFAR setup

* Removing old results in AMI

* More work on nnet3-egs-augment-image.cc

* [build] Slight change to how tests are reported, to figure out which one is not completing.

* Add data preparation script for CIFAR

* Add cmd.sh and run.sh

* Various fixes to CIFAR setup

* [src] Code fix RE compressed matrices
kronos-cm added a commit to kronos-cm/kaldi that referenced this pull request Apr 28, 2017
* 'master' of https://github.com/kaldi-asr/kaldi: (21 commits)
  [egs] bug-fix in egs/ami/s5/run_ihm.sh (kaldi-asr#1577)
  [src] Minor bug-fixes in compute-wer-bootci and WSJ run.sh.  Thanks: @osadjadi
  [egs] Add soft link for mini-librispeech setup
  [egs] adding results and cleanup in mini-librispeech
  [egs] Add mini-librispeech example scripts [intended as a sanity-checker/tutorial setup] (kaldi-asr#1566)
  [src] Fix to testing code signal-test.cc, change threshold to resolve failure (kaldi-asr#1565)
  [src] Add documentation for dropout function.
  [src,scripts,egs]  Add dropout for nnet3 LSTMs, with recipes. (kaldi-asr#1537)
  [src] nnet3 online silence weighting - adding frame subsampling factor (kaldi-asr#1559)
  [doc] Small edit to hmm.dox, clarifying something
  [egs] Added check for kaldi_lm being installed in fisher_swbd recipe. (kaldi-asr#1558)
  Update travis.yml so PRs to kaldi_52 are built
  [srcipts] steps/nnet3/report/generate_plots.py: plot 5,50,95th percentile of value and derivative instead of mean+-stddev (kaldi-asr#1472)
  [egs] AMI TDNN Results Update (kaldi-asr#1545)
  [src] add template instantiations for ConvertStringToReal, address issue kaldi-asr#1544
  [egs,scripts,src] SID and LID tools and scripts: cosmetic improvements, better error-handling, and various minor fixes; results unchanged. (kaldi-asr#1543)
  [src] Change ConvertStringToReal to be locale-independent (i.e. always-US).  Fixes android issue. (kaldi-asr#1513)
  [scripts] nnet3 : fix issue where LDA estimation failed for LSTMs with label delay (kaldi-asr#1540)
  [scripts] fix to get_egs_targets.sh (thanks: David Pye)
  [src] Fix copy-feats for using the --write-num-frames and --compress true flags at the same time (kaldi-asr#1541)
  ...
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
…tile of value and derivative instead of mean+-stddev (kaldi-asr#1472)
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
* [srcipts] steps/nnet3/report/generate_plots.py: plot 5,50,95th percentile of value and derivative instead of mean+-stddev (kaldi-asr#1472)

* Update travis.yml so PRs to kaldi_52 are built

* Setting up basic structure for CIFAR directory.

* [src] Some code changes/additions to support image recognition applications of nnet3

* Adding results for using batchnorm components instead of renorm

* Some partial work on CIFAR setup

* Removing old results in AMI

* More work on nnet3-egs-augment-image.cc

* [build] Slight change to how tests are reported, to figure out which one is not completing.

* Add data preparation script for CIFAR

* Add cmd.sh and run.sh

* Various fixes to CIFAR setup

* [src] Code fix RE compressed matrices
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.

None yet

3 participants