-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Use KALDI_ROOT env if it's initialized #1932
Use KALDI_ROOT env if it's initialized #1932
Conversation
egs/csj/s5/local/csj_train_lms.sh
Outdated
@@ -50,9 +50,9 @@ done | |||
loc=`which ngram-count`; | |||
if [ -z $loc ]; then | |||
if uname -a | grep 64 >/dev/null; then # some kind of 64 bit... | |||
sdir=`pwd`/../../../tools/srilm/bin/i686-m64 | |||
sdir=${KALDI_ROOT:-`pwd`/../../..}/tools/srilm/bin/i686-m64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it would make more sense to have the script source path.sh
(. ./path.sh
)
and then jsut use ${KALDI_ROOT} with no backoff to ../../../.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your advice.
Looks good to me.
Should I squash commits? |
egs/csj/s5/local/csj_train_lms.sh
Outdated
@@ -42,6 +42,7 @@ fi | |||
text=$1 | |||
lexicon=$2 | |||
dir=$3 | |||
. path.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do . ./path.sh
(in case there is a path.sh in a directory on the current PATH, or . is not on the path).
no need to squash
I'll merge to-morrow
Previous script, used relative PATH that used
pwd
.When cloning only CSJ-recipe for tuning it, it is hard to keep the directory structure.
IMO, we should use KALDI_ROOT env that initialized in
path.sh
instead ofpwd
's relative path.