-
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
update fisher_swbd recipe #1498
Conversation
tools/extras/install_srilm.sh
Outdated
@@ -14,7 +14,7 @@ if [ ! -d liblbfgs-1.10 ]; then | |||
fi | |||
|
|||
# http://www.speech.sri.com/projects/srilm/download.html | |||
if [ ! -f srilm.tgz ]; then | |||
if [ ! -f srilm-1.7.2.tar ]; then |
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.
@naxingyu , I won't comment on the recipe changes per se (not familiar with that)
But I'm not fond of the changes here.
a) what srilm pages do offer for download if you go there is srilm-1.7.2.tar.gz, i.e. I'm not sure where the tar file comes from, but that script won't work for other people now.
b) I'm still in favor of asking users to do a symlink or copy the file as srilm.tgz.
I think it makes sense as we cannot download the file automatically -- so people can use whatever version they have available and do not have to undergo the downloading process every time. Which I realize it's not true now as I'm thinking about it, as we need 1.7.1 at least, because of the maxent LMs. So I don't really object to having a fixed version and bump it now and then, but I think it's less convenient for us.
c) "$SRILM" is preferred, IMO.
It's because of cases where the users have space (white character) in the path somewhere. Or perhaps not, but is there any rationale behind changing it?
在 2017年3月17日,23:58,jtrmal ***@***.***> 写道:
@jtrmal commented on this pull request.
In tools/extras/install_srilm.sh <#1498 (comment)>:
> @@ -14,7 +14,7 @@ if [ ! -d liblbfgs-1.10 ]; then
fi
# http://www.speech.sri.com/projects/srilm/download.html
-if [ ! -f srilm.tgz ]; then
+if [ ! -f srilm-1.7.2.tar ]; then
@naxingyu <https://github.com/naxingyu> , I won't comment on the recipe changes per se (not familiar with that)
But I'm not fond of the changes here.
a) what srilm pages do offer for download if you go there is srilm-1.7.2.tar.gz, i.e. I'm not sure where the tar file comes from, but that script won't work for other people now.
The tar file is what I got from the download page, last month. I don’t if they have changed the offered file from tgz to tar, but that’s what I got.
b) I'm still in favor of asking users to do a symlink or copy the file as srilm.tgz.
I think it makes sense as we cannot download the file automatically -- so people can use whatever version they have available and do not have to undergo the downloading process every time. Which I realize it's not true now as I'm thinking about it, as we need 1.7.1 at least, because of the maxent LMs. So I don't really object to having a fixed version and bump it now and then, but I think it's less convenient for us.
c) "$SRILM" is preferred, IMO.
I agree. Will change it back.
… It's because of cases where the users have space (white character) in the path somewhere. Or perhaps not, but is there any rationale behind changing it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#1498 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADKpxJ5ZfhBIIqv5VCw8TsRyBIDBVhr3ks5rmq2rgaJpZM4MgSPH>.
|
Check with a couple ways of getting it, maybe some browsers or OS's
automatically unzip things.
…On Fri, Mar 17, 2017 at 9:07 PM, Xingyu Na ***@***.***> wrote:
> 在 2017年3月17日,23:58,jtrmal ***@***.***> 写道:
>
> @jtrmal commented on this pull request.
>
> In tools/extras/install_srilm.sh <https://github.com/kaldi-asr/
kaldi/pull/1498#discussion_r106684126>:
>
> > @@ -14,7 +14,7 @@ if [ ! -d liblbfgs-1.10 ]; then
> fi
>
> # http://www.speech.sri.com/projects/srilm/download.html
> -if [ ! -f srilm.tgz ]; then
> +if [ ! -f srilm-1.7.2.tar ]; then
> @naxingyu <https://github.com/naxingyu> , I won't comment on the recipe
changes per se (not familiar with that)
> But I'm not fond of the changes here.
> a) what srilm pages do offer for download if you go there is
srilm-1.7.2.tar.gz, i.e. I'm not sure where the tar file comes from, but
that script won't work for other people now.
>
The tar file is what I got from the download page, last month. I don’t if
they have changed the offered file from tgz to tar, but that’s what I got.
> b) I'm still in favor of asking users to do a symlink or copy the file
as srilm.tgz.
> I think it makes sense as we cannot download the file automatically --
so people can use whatever version they have available and do not have to
undergo the downloading process every time. Which I realize it's not true
now as I'm thinking about it, as we need 1.7.1 at least, because of the
maxent LMs. So I don't really object to having a fixed version and bump it
now and then, but I think it's less convenient for us.
> c) "$SRILM" is preferred, IMO.
>
I agree. Will change it back.
> It's because of cases where the users have space (white character) in
the path somewhere. Or perhaps not, but is there any rationale behind
changing it?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <
#1498 (review)>,
or mute the thread <https://github.com/notifications/unsubscribe-auth/
ADKpxJ5ZfhBIIqv5VCw8TsRyBIDBVhr3ks5rmq2rgaJpZM4MgSPH>.
>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1498 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADJVuxgsXPeop0EocRZWQsWiZpkXL3w1ks5rmy5ugaJpZM4MgSPH>
.
|
You are right… I’ll change the extension. And do I keep the version numbers or not?
… 在 2017年3月18日,09:12,Daniel Povey ***@***.***> 写道:
Check with a couple ways of getting it, maybe some browsers or OS's
automatically unzip things.
On Fri, Mar 17, 2017 at 9:07 PM, Xingyu Na ***@***.***> wrote:
>
> > 在 2017年3月17日,23:58,jtrmal ***@***.***> 写道:
> >
> > @jtrmal commented on this pull request.
> >
> > In tools/extras/install_srilm.sh <https://github.com/kaldi-asr/
> kaldi/pull/1498#discussion_r106684126>:
> >
> > > @@ -14,7 +14,7 @@ if [ ! -d liblbfgs-1.10 ]; then
> > fi
> >
> > # http://www.speech.sri.com/projects/srilm/download.html
> > -if [ ! -f srilm.tgz ]; then
> > +if [ ! -f srilm-1.7.2.tar ]; then
> > @naxingyu <https://github.com/naxingyu> , I won't comment on the recipe
> changes per se (not familiar with that)
> > But I'm not fond of the changes here.
> > a) what srilm pages do offer for download if you go there is
> srilm-1.7.2.tar.gz, i.e. I'm not sure where the tar file comes from, but
> that script won't work for other people now.
> >
> The tar file is what I got from the download page, last month. I don’t if
> they have changed the offered file from tgz to tar, but that’s what I got.
> > b) I'm still in favor of asking users to do a symlink or copy the file
> as srilm.tgz.
> > I think it makes sense as we cannot download the file automatically --
> so people can use whatever version they have available and do not have to
> undergo the downloading process every time. Which I realize it's not true
> now as I'm thinking about it, as we need 1.7.1 at least, because of the
> maxent LMs. So I don't really object to having a fixed version and bump it
> now and then, but I think it's less convenient for us.
> > c) "$SRILM" is preferred, IMO.
> >
> I agree. Will change it back.
> > It's because of cases where the users have space (white character) in
> the path somewhere. Or perhaps not, but is there any rationale behind
> changing it?
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub <
> #1498 (review)>,
> or mute the thread <https://github.com/notifications/unsubscribe-auth/
> ADKpxJ5ZfhBIIqv5VCw8TsRyBIDBVhr3ks5rmq2rgaJpZM4MgSPH>.
>
> >
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#1498 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/ADJVuxgsXPeop0EocRZWQsWiZpkXL3w1ks5rmy5ugaJpZM4MgSPH>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#1498 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADKpxErNlWoxchB600GY5HlGzSh409aBks5rmy9tgaJpZM4MgSPH>.
|
Let's not keep version numbers, it will be more compatible with existing
setups.
…On Fri, Mar 17, 2017 at 9:15 PM, Xingyu Na ***@***.***> wrote:
You are right… I’ll change the extension. And do I keep the version
numbers or not?
> 在 2017年3月18日,09:12,Daniel Povey ***@***.***> 写道:
>
> Check with a couple ways of getting it, maybe some browsers or OS's
> automatically unzip things.
>
>
> On Fri, Mar 17, 2017 at 9:07 PM, Xingyu Na ***@***.***>
wrote:
>
> >
> > > 在 2017年3月17日,23:58,jtrmal ***@***.***> 写道:
> > >
> > > @jtrmal commented on this pull request.
> > >
> > > In tools/extras/install_srilm.sh <https://github.com/kaldi-asr/
> > kaldi/pull/1498#discussion_r106684126>:
> > >
> > > > @@ -14,7 +14,7 @@ if [ ! -d liblbfgs-1.10 ]; then
> > > fi
> > >
> > > # http://www.speech.sri.com/projects/srilm/download.html
> > > -if [ ! -f srilm.tgz ]; then
> > > +if [ ! -f srilm-1.7.2.tar ]; then
> > > @naxingyu <https://github.com/naxingyu> , I won't comment on the
recipe
> > changes per se (not familiar with that)
> > > But I'm not fond of the changes here.
> > > a) what srilm pages do offer for download if you go there is
> > srilm-1.7.2.tar.gz, i.e. I'm not sure where the tar file comes from,
but
> > that script won't work for other people now.
> > >
> > The tar file is what I got from the download page, last month. I don’t
if
> > they have changed the offered file from tgz to tar, but that’s what I
got.
> > > b) I'm still in favor of asking users to do a symlink or copy the
file
> > as srilm.tgz.
> > > I think it makes sense as we cannot download the file automatically
--
> > so people can use whatever version they have available and do not have
to
> > undergo the downloading process every time. Which I realize it's not
true
> > now as I'm thinking about it, as we need 1.7.1 at least, because of the
> > maxent LMs. So I don't really object to having a fixed version and
bump it
> > now and then, but I think it's less convenient for us.
> > > c) "$SRILM" is preferred, IMO.
> > >
> > I agree. Will change it back.
> > > It's because of cases where the users have space (white character) in
> > the path somewhere. Or perhaps not, but is there any rationale behind
> > changing it?
> > >
> > > —
> > > You are receiving this because you were mentioned.
> > > Reply to this email directly, view it on GitHub <
> > #1498#
pullrequestreview-27621733>,
> > or mute the thread <https://github.com/notifications/unsubscribe-auth/
> > ADKpxJ5ZfhBIIqv5VCw8TsRyBIDBVhr3ks5rmq2rgaJpZM4MgSPH>.
> >
> > >
> >
> > —
> > You are receiving this because you are subscribed to this thread.
> > Reply to this email directly, view it on GitHub
> > <#1498 (comment)>,
or mute
> > the thread
> > <https://github.com/notifications/unsubscribe-auth/
ADJVuxgsXPeop0EocRZWQsWiZpkXL3w1ks5rmy5ugaJpZM4MgSPH>
> > .
> >
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <
#1498 (comment)>, or
mute the thread <https://github.com/notifications/unsubscribe-auth/
ADKpxErNlWoxchB600GY5HlGzSh409aBks5rmy9tgaJpZM4MgSPH>.
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1498 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADJVu-w6l4i9JnaCpAR1l1hq756OIUziks5rmzBOgaJpZM4MgSPH>
.
|
OK. I’ve reverted the changes on installation script.
… 在 2017年3月18日,09:18,Daniel Povey ***@***.***> 写道:
Let's not keep version numbers, it will be more compatible with existing
setups.
On Fri, Mar 17, 2017 at 9:15 PM, Xingyu Na ***@***.***> wrote:
> You are right… I’ll change the extension. And do I keep the version
> numbers or not?
>
>
> > 在 2017年3月18日,09:12,Daniel Povey ***@***.***> 写道:
> >
> > Check with a couple ways of getting it, maybe some browsers or OS's
> > automatically unzip things.
> >
> >
> > On Fri, Mar 17, 2017 at 9:07 PM, Xingyu Na ***@***.***>
> wrote:
> >
> > >
> > > > 在 2017年3月17日,23:58,jtrmal ***@***.***> 写道:
> > > >
> > > > @jtrmal commented on this pull request.
> > > >
> > > > In tools/extras/install_srilm.sh <https://github.com/kaldi-asr/
> > > kaldi/pull/1498#discussion_r106684126>:
> > > >
> > > > > @@ -14,7 +14,7 @@ if [ ! -d liblbfgs-1.10 ]; then
> > > > fi
> > > >
> > > > # http://www.speech.sri.com/projects/srilm/download.html
> > > > -if [ ! -f srilm.tgz ]; then
> > > > +if [ ! -f srilm-1.7.2.tar ]; then
> > > > @naxingyu <https://github.com/naxingyu> , I won't comment on the
> recipe
> > > changes per se (not familiar with that)
> > > > But I'm not fond of the changes here.
> > > > a) what srilm pages do offer for download if you go there is
> > > srilm-1.7.2.tar.gz, i.e. I'm not sure where the tar file comes from,
> but
> > > that script won't work for other people now.
> > > >
> > > The tar file is what I got from the download page, last month. I don’t
> if
> > > they have changed the offered file from tgz to tar, but that’s what I
> got.
> > > > b) I'm still in favor of asking users to do a symlink or copy the
> file
> > > as srilm.tgz.
> > > > I think it makes sense as we cannot download the file automatically
> --
> > > so people can use whatever version they have available and do not have
> to
> > > undergo the downloading process every time. Which I realize it's not
> true
> > > now as I'm thinking about it, as we need 1.7.1 at least, because of the
> > > maxent LMs. So I don't really object to having a fixed version and
> bump it
> > > now and then, but I think it's less convenient for us.
> > > > c) "$SRILM" is preferred, IMO.
> > > >
> > > I agree. Will change it back.
> > > > It's because of cases where the users have space (white character) in
> > > the path somewhere. Or perhaps not, but is there any rationale behind
> > > changing it?
> > > >
> > > > —
> > > > You are receiving this because you were mentioned.
> > > > Reply to this email directly, view it on GitHub <
> > > #1498#
> pullrequestreview-27621733>,
> > > or mute the thread <https://github.com/notifications/unsubscribe-auth/
> > > ADKpxJ5ZfhBIIqv5VCw8TsRyBIDBVhr3ks5rmq2rgaJpZM4MgSPH>.
> > >
> > > >
> > >
> > > —
> > > You are receiving this because you are subscribed to this thread.
> > > Reply to this email directly, view it on GitHub
> > > <#1498 (comment)>,
> or mute
> > > the thread
> > > <https://github.com/notifications/unsubscribe-auth/
> ADJVuxgsXPeop0EocRZWQsWiZpkXL3w1ks5rmy5ugaJpZM4MgSPH>
> > > .
> > >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub <
> #1498 (comment)>, or
> mute the thread <https://github.com/notifications/unsubscribe-auth/
> ADKpxErNlWoxchB600GY5HlGzSh409aBks5rmy9tgaJpZM4MgSPH>.
> >
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#1498 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/ADJVu-w6l4i9JnaCpAR1l1hq756OIUziks5rmzBOgaJpZM4MgSPH>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#1498 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADKpxNopo4-FtebdIzr0xePIYd3zJgtYks5rmzDpgaJpZM4MgSPH>.
|
|
||
kaldi_lm=`which train_lm.sh` | ||
if [ ! -x $kaldi_lm ]; then | ||
echo "train_lm.sh is not found. Checkout tools/extra/install_kaldi_lm.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.
Say "look at" not "checkout".
6375c1d
to
e0b8aac
Compare
Changed and squashed.
… 在 2017年3月18日,09:27,Daniel Povey ***@***.***> 写道:
@danpovey commented on this pull request.
In egs/fisher_swbd/s5/local/fisher_train_lms.sh <#1498 (comment)>:
>
+kaldi_lm=`which train_lm.sh`
+if [ ! -x $kaldi_lm ]; then
+ echo "train_lm.sh is not found. Checkout tools/extra/install_kaldi_lm.sh"
Say "look at" not "checkout".
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#1498 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADKpxHIJMklde-LQR7tukrbog1_cl0f2ks5rmzL_gaJpZM4MgSPH>.
|
Merging. Thanks! |
update fisher_swbd to adopt new path and srilm installer.