search

kaldi-asr / kaldi

kaldi-asr/kaldi is the official location of the Kaldi project.
http://kaldi-asr.org
Other
14.31k stars 5.32k forks source link

check error for "shift 2" in parse argument to avoid infinite loop #3998

Open itaipee opened 4 years ago

itaipee commented 4 years ago

in case the last argument is variable name ,parse_opts might get stuck in infinite lop , for example "./steps/nnet2/train_pnorm.sh --cmd run.pl --lda-opts"
the problem is that "shift 2" command fails and the "--lda-opts" is still first argument $1.

Suggestion for fix
! shift 2 && echo "$0: no option for $name" 1>&2 && exit 1;

Since this is very basic script that is used everywhere , I'm not sure it is wise to change it

https://github.com/kaldi-asr/kaldi/blob/53c4bd19f8a9f5f2ec7280b00a0df8720b692cd9/egs/wsj/s5/utils/parse_options.sh#L85

danpovey commented 4 years ago

Your proposed fix seems fine. Could you do some basic testing and make a PR?

itaipee commented 4 years ago

sure , is there any check-list for PR ? like regression or something i need to do beside the actual code change ? thanks , Itai

On Tue, Mar 17, 2020 at 4:30 AM Daniel Povey notifications@github.com wrote:

Your proposed fix seems fine. Could you do some basic testing and make a PR?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/3998#issuecomment-599845596, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALAMETEY2R4NVCVA7BNG2YLRH3OCXANCNFSM4LMMIT5Q .

-- Itai Peer

jtrmal commented 4 years ago

Not really.... You can run minilibrispeech (or yesno, if you want something extremely tiny) y.

On Wed, Mar 18, 2020 at 8:26 AM itaipee notifications@github.com wrote:

sure , is there any check-list for PR ? like regression or something i need to do beside the actual code change ? thanks , Itai

On Tue, Mar 17, 2020 at 4:30 AM Daniel Povey notifications@github.com wrote:

Your proposed fix seems fine. Could you do some basic testing and make a PR?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/3998#issuecomment-599845596, or unsubscribe < https://github.com/notifications/unsubscribe-auth/ALAMETEY2R4NVCVA7BNG2YLRH3OCXANCNFSM4LMMIT5Q

.

-- Itai Peer

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kaldi-asr/kaldi/issues/3998#issuecomment-600592573, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACUKYXZY4G7V7MI44MLRLCTRIC4V5ANCNFSM4LMMIT5Q .

stale[bot] commented 4 years ago

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.