Skip to content

Python script to convert nnet2 to nnet3 models #1611

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

Merged
merged 4 commits into from
Jun 7, 2017

Conversation

jfainberg
Copy link
Contributor

Re #886 .

Initial implementation - I'll test it a bit more over the next few days, but open for comments.

Couple of points:

  • Currently considers everything but the final model temporary (including the generated config it uses to initialise the model).
  • Uses numpy to combine matrices and vectors into a single matrix for initialisation. Possible to do using just file manipulation, just a bit more messy.
  • Ignores ValueSum and DerivSum stat fields of e.g. normalize and softmax components.

@vijayaditya
Copy link
Contributor

"Uses numpy to combine matrices and vectors into a single matrix for initialisation. Possible to do using just file manipulation, just a bit more messy."

I think this dependence on numpy for cleaner code is fine, as this binary is going to be used by people who might have sufficient experience with Kaldi.

KNOWN_COMPONENTS = NODE_NAMES.keys()
# End configuration section

logger = logging.getLogger(__name__)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not familiar with the latest logging guidelines. @vimalmanohar could you take a look.

Copy link
Contributor

Choose a reason for hiding this comment

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

The logging is fine. Although, the guidelines for variable names and spacing in the Google style guide are not followed. https://google.github.io/styleguide/pyguide.html

Copy link
Contributor

@vijayaditya vijayaditya left a comment

Choose a reason for hiding this comment

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

Here are some comments after a quick initial review.

if (result != 0):
raise OSError('Encountered an error writing the model.')

def ParseNnet2(line_buffer):
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to rename this as ParseNnet2ToNnet3


def ConsumeToken(token, line):
'''Returns line without token'''
if token != line.split()[0]:
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC these lines can be very large. So it might be better to just check for this using regexes or at least using split() method using something like a maxsplit=1 option.


def WriteModel(self, model, binary="true"):
result = 0

Copy link
Contributor

Choose a reason for hiding this comment

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

Check that self.config is a proper nnet3 config file.

self.priors,
model), shell=True)

if (result != 0):
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 it might be worth adding a check in a top level shell script which performs forward prop through the nnet2 and nnet3 models and asserts that the values are within acceptable threshold ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if there is a easy way to check the correctness of the transition model without doing a decode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. How about another argument to the argparser for some features, and if present, will run a forward pass and compare the results using difflib; so keeping it all in the single Python script?

The transition model isn't read by Numpy, so shouldn't have any numerical errors I think.

result = 0

# write raw model
result += subprocess.call('nnet3-init --binary=true {0} {1}'.format(self.config, os.path.join(tmpdir, 'nnet3.raw')), shell=True)
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 want to consider using the KaldiCommand function available in the nnet3 python libraries ? This might help you handle some common errors.

for component in self.components:
if component.ident == "splice":
# Create splice string for the next node
previous_component=MakeSpliceString(previous_component, component.pairs['<Context>'])
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not clear how you are handling the SpliceMaxComponent here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I had forgot about SpliceMaxComponent - it's not explicitly handled at the minute. What is the equivalent in nnet3? I couldn't find a max-descriptor (e.g. 'Max(Offset, ...)')

@@ -0,0 +1,441 @@
#!/usr/bin/env python
# Copyright 2016

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add your name to the author list.

Function/method names, spaces, column width.
@vijayaditya
Copy link
Contributor

vijayaditya commented May 9, 2017 via email

@danpovey
Copy link
Contributor

danpovey commented Jun 7, 2017

So @jfainberg, this is ready to commit? Still says WIP.

@jfainberg jfainberg changed the title WIP: Python script to convert nnet2 to nnet3 models Python script to convert nnet2 to nnet3 models Jun 7, 2017
@danpovey
Copy link
Contributor

danpovey commented Jun 7, 2017

@vijayaditya if you say it's still good I'll merge.

@jfainberg
Copy link
Contributor Author

Yes @danpovey, please go ahead. I've tested it (forward pass) with typical nnet2 pnorm and tanh networks.

@vijayaditya
Copy link
Contributor

@danpovey Went through it briefly. LGTM.

@danpovey danpovey merged commit a0795ec into kaldi-asr:master Jun 7, 2017
kronos-cm added a commit to kronos-cm/kaldi that referenced this pull request Jun 15, 2017
* 'master' of https://github.com./kaldi-asr/kaldi: (140 commits)
  [egs] Fix failure in multilingual BABEL recipe (regenerate cmvn.scp) (kaldi-asr#1686)
  [src,scripts,egs] Backstitch code+scripts, and one experiment, will add more later. (kaldi-asr#1605)
  [egs] CNN+TDNN+LSTM experiments on AMI (kaldi-asr#1685)
  [egs,scripts,src] Tune image recognition examples; minor small changes. (kaldi-asr#1682)
  [src] Fix bug in looped computation (kaldi-asr#1673)
  [build] when installing sequitur and mmseg, look for lib64 as well (thanks: @akshayc11) (kaldi-asr#1677)
  [src] fix to gst-plugin/Makefile (remove -lkaldi-thread) (kaldi-asr#1680)
  [src] Cosmetic fixes to usage messages
  [egs]  Fix to some --proportional-shrink related example scripts (kaldi-asr#1674)
  [build] Fix small bug in configure
  [scripts] Fix small bug in utils/gen_topo.pl.
  [scripts] Add python script to convert nnet2 to nnet3 models (kaldi-asr#1611)
  [doc] Fix typo  (kaldi-asr#1669)
  [src] nnet3: fix small bug in checking code.  Thanks: @Maddin2000.
  [src] Add #include missing from previous commit
  [src] Fix bug in online2-nnet3 decoding RE dropout+batch-norm (thanks: Wonkyum Lee)
  [scripts] make errors getting report non-fatal (thx: Miguel Jette); add comment RE dropout proportion
  [src,scripts] Use ConstFst or decoding (half the memory; slightly faster).  (kaldi-asr#1661)
  [src] keyword search tools: fix Minimize() call, necessary due to OpenFst upgrade (kaldi-asr#1663)
  [scripts] do not fail if the ivector extractor belongs to different user (kaldi-asr#1662)
  ...
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.

4 participants