Add option --output-name to nnet3-compute{,-batch}

[kkm000]

--output-name : Do computation for specified output-node (string, default = "output")

The user had a network with multiple output-nodes. His life would have been easier if we had this option.

[kkm000]

@danpovey, I wanted to run a quick design question by you, not urgent, can wait. All structs in the Nnet*ComputationOptions hierarchy have the word 'Simple' in it (I know what it signifies). I could derive one from it, no longer "Simple" to specify the output node name. But there is another quirk that I would like to get rid of at the same time, whle I'm on it: the topmost one in this hierarchy, NnetSimpleComputationOptions, has the acoustic_scale field, which is carried over to all programs with a puzzling at first string for the --help message "not used if this program is nnet3-compute." And since it's now also unused if "this program" is nnet3-compute-batch, this asks for the extending of this kind of commentary.

The naming looks like you had been thinking also about the options for a non-Simple case from the start, but this then never materialized.

So, my fingers are really itchy to rename the struct NnetSimpleComputationOptions to NnetSimpleAmComputationOptions, that would add only the acoustic_weight field to its base class, which will be the the NnetSimpleComputationOptions (same name, one field taken out). From this one, I can derive the options classes for nnet3-compute and nnet3-compute-batch. These would be w/o the unused switch/field, and also not "Simple", as they would specify the output node name (without the word "Simple" in the name).

There is a simpler way to hide the unused switch, an additional argument to Register namely, but it would be out of pattern of the use of Register elsewhere; the inheritance approach seems squeaky clean to me. Do you see any downside in this? And, if you ok my proposed change, is the name NnetSimpleAmComputationOptions ok? It's used in quite more than a few places, so I do not want to have to rename it twice. The AM cannot be non-Simple, so this is not prompting for a future possible split.

[danpovey]

I'm OK with doing the hierarchy thing, and with those names. The name "Simple" didn't turn out to be that helpful, but I'd rather not change things too much as it might confuse people.

On Sun, Apr 7, 2019 at 4:33 AM kkm (aka Kirill Katsnelson) < notifications@github.com> wrote:

@danpovey https://github.com/danpovey, I wanted to run a quick design question by you, not urgent, can wait. All structs in the Nnet*ComputationOptions hierarchy have the word 'Simple' in it (I know what it signifies). I could derive one from it, no longer "Simple" to specify the output node name. But there is another quirk that I would like to get rid of at the same time, whle I'm on it: the topmost one in this hierarchy, NnetSimpleComputationOptions, has the acoustic_scale field, which is carried over to all programs with a puzzling at first string for the --help message "not used if this program is nnet3-compute." And since it's now also unused if "this program" is nnet3-compute-batch, this asks for the extending of this kind of commentary.

The naming looks like you had been thinking also about the options for a non-Simple case from the start, but this then never materialized.

So, my fingers are really itchy to rename the struct NnetSimpleComputationOptions to NnetSimpleAmComputationOptions, that would add only the acoustic_weight field to its base class, which will be the the NnetSimpleComputationOptions (same name, one field taken out). From this one, I can derive the options classes for nnet3-compute and nnet3-compute-batch. These would be w/o the unused switch/field, and also not "Simple", as they would specify the output node name (without the word "Simple" in the name).

There is a simpler way to hide the unused switch, an additional argument to Register namely, but it would be out of pattern of the use of Register elsewhere; the inheritance approach seems squeaky clean to me. Do you see any downside in this? And, if you ok my proposed change, is the name NnetSimpleAmComputationOptions ok? It's used in quite more than a few places, so I do not want to have to rename it twice. The AM cannot be non-Simple, so this is not prompting for a future possible split.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/3193#issuecomment-480582034, or mute the thread https://github.com/notifications/unsubscribe-auth/ADJVuy0pZMeGd0jEAZnwc7oY6Y21R5IFks5veddsgaJpZM4cVOUa .

[kkm000]

The most global one would be NnetSimpleComputationOptions -> NnetSimpleAmComputationOptions. I'll make sure to add comments to the struct that will be reported as not having the member by the compiler.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.