kaldi
kaldi copied to clipboard
danpovey
Changes from vijay to the xconfig branch.
- Moved library files to a sub-directory as we will have more library files in PR 1066
If you move them like that, perhaps you could rename them to e.g. xconfig/utils.py and xconfig/layers.py, and import as xconfig.utils and xconfig.layers, if you feel that's more Pythonic.
On Mon, Nov 7, 2016 at 7:45 PM, Vijayaditya Peddinti < [email protected]> wrote:
- Moved library files to a sub-directory as we will have more library files in PR 1066
You can view, comment on, or merge this pull request online at:
https://github.com/danpovey/kaldi/pull/30 Commit Summary
- Moved library files to a sub-directory as we will have more library
File Changes
- R egs/wsj/s5/steps/nnet3/libs/xconfig/xconfig_layers.py https://github.com/danpovey/kaldi/pull/30/files#diff-0 (0)
- R egs/wsj/s5/steps/nnet3/libs/xconfig/xconfig_utils.py https://github.com/danpovey/kaldi/pull/30/files#diff-1 (0)
Patch Links:
- https://github.com/danpovey/kaldi/pull/30.patch
- https://github.com/danpovey/kaldi/pull/30.diff
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/danpovey/kaldi/pull/30, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVuxZ17tOlGH14IBGeOgGQP9S_hOxtks5q78ZDgaJpZM4Kr5ye .
@vimalmanohar I have created the package structure according to our discussion.
@freewym Would you be able to help in this PR ? You might be maintaining these scripts in the future so it would be good to familiarize yourself.
Could you look at the LSTMP xconfig and write one for the normal LSTM (i.e., without the projection layers) ?
@vijayaditya I made a PR to your repo to add an LSTM's xconfig layer (w/o projection)
@danpovey could you check the latest commit and let me know if you agree with the Nones class that I am using.
This was a hacky way to avoid explicitly specifying the type of config variable when we don't know what the default has to be and want the C++ code to use the default it wants.
Regarding the Nones stuff... I think negative values would normally be easier to follow. The python could just, say, not print out the config value in the config file if the source was -1, or unset.
The negative values would not work when we would like to specify things like means (we support bias-mean in NaturalGradientAffineComponent), so I thought this was a consistent way to just say we don't what this value is.
Sorry, I think it's going to make the code harder for newbies to understand. I'd rather keep it obvious... e.g. in that case of the bias-mean [which it's not clear we would ever need to expose at the xconfig level], you could just leave the default at zero. For cases where we want to, say, set obscure options like natural-gradient options, we could have a config like natural-gradient-opts='input-rank=20 output-rank=40 alpha=2.0' (this would involve changing the parsing code to be able to parse quoted strings with ='s in them, which is not hard). If there is a case where you really need to have an unset value for a particular config value, there will usually be a particular value that doesn't make sense, or that's going to remain the C++ default, that you could use as the default, like 0 or -1. This may be ugly, but at least the ugliness is highly localized. The advantage of 'dim=-1' is that any idiot can see what it means.
Also, remember that the script prints out the 'xconfig.expanded', and your nones would appear there and would look rather ugly. I'd rather have more human-interpretable unset values. [and note: if we eventually allow strings with = in them, they'd have to be printed quoted in xconfig.expanded.]
It would be nice to see your example scripts that you are using for testing.
BTW, if you make this a PR to the Kaldi repo it would be better, then others (not just me) will see it.