[tools, extras] morfessor installation script #2299

lucas-jo · 2018-03-22T03:17:11Z

added install_morfessor.sh and its symbolic link

jtrmal · 2018-03-22T03:20:14Z

tools/extras/install_morfessor.sh

+echo "#### installing morfessor"
+dirname=morfessor
+if [ ! -d ./$dirname ]; then
+	mkdir -p ./$dirname


could you please change the tabs to spaces (each tab = two spaces)?

applied retab with size 2

jtrmal · 2018-03-22T03:21:17Z

tools/install_morfessor.sh

@@ -0,0 +1 @@
+extras/install_morfessor.sh


I think you don't have to add this symlink (that is done only for historical reasons for some packages)

okay got it. I deleted the symlink

jtrmal · 2018-03-22T03:25:18Z

tools/extras/install_morfessor.sh

+fi
+
+# local installation
+site_packages_dir=$(python -m site --user-site | grep -oE "lib.*")


typically we try to not to install the packages system-wise (user-wise) -- we keep the libs in kaldi/tools/ and just set up the shell variables in env.sh (PYTHONPATH). Otherwise, you could just use pip install morfessor.

Also, in that case, the uninstallation is not needed -- you just delete the directory.

@jtrmal
That line is not intended to install morfessor in user-wise way.
It is just to grep the name of site-package name and use the name for PYTHONPATH variable before installation started,
otherwise, sometimes installation process could get an error.

Anyway after installation process, the morfessor libs will be located inside kaldi/tools/

danpovey · 2018-03-24T22:57:01Z

@jtrmal, please merge when you think it's OK.

jtrmal · 2018-03-26T01:30:39Z

Yep, I will get to this tmrw, I'm still not fully comfortable with my understanding how the script works. Y.

…

On Sat, Mar 24, 2018, 18:57 Daniel Povey ***@***.***> wrote: @jtrmal <https://github.com/jtrmal>, please merge when you think it's OK. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2299 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKisX7SPJ0Nuw2EjIsBF-6xMdOk0oY5Mks5ths9EgaJpZM4S2Y5E> .

jtrmal · 2018-03-26T13:32:42Z

tools/extras/install_morfessor.sh

+  mkdir -p ./$dirname
+  git clone https://github.com/aalto-speech/morfessor.git morfessor ||
+    {
+      if [ $? -ne 0 ]; then


this is confusing -- why you are testing the return code if you have the guarantee done by ||

it should be removed

jtrmal · 2018-03-26T13:38:43Z

tools/extras/install_morfessor.sh

+wd=`pwd`
+wd=`readlink -f $wd || pwd`
+export MORFESSOR="$wd/morfessor"
+export PYTHONPATH="${PYTHONPATH:-}:$MORFESSOR/${site_packages_dir}"


I still think this is too complex for no clear reason (reason clear to me) -- you could just install into $MORFESSOR/local/ (or whatever) and this would greatly simplify the script.

@jtrmal yep I agree on it.
Anyway it seems like @psmit shows us more clear way.
After scripts in scripts/ has been changed to executables, I'll update this code in more simpler way.

jtrmal · 2018-03-26T13:40:22Z

tools/extras/install_morfessor.sh

+
+echo >&2 "installation of MORFESSOR finished successfully"
+echo >&2 "please source tools/env.sh in your path.sh to enable it"
+echo >&2 'when uninstall it, try: sudo rm $(cat ./morfessor/files.txt)'


and when uninstalling -- you should delete the whole $MORFESSOR fir (and sudo shouldn't be needed to be used?)

It was intended to inform where exist the real files. Will be removed either

psmit · 2018-03-26T15:02:57Z

Morfessor author here. We normally recommend installing through pip or conda, but in this case there is a much simpler way:

Clone the github
Set PYTHONPATH to include the repo
Set PATH to include the scripts directory

Running setup.py has no benefit here in my opinion.

Uninstalling is as simple as removing the repo (and cleaning up the variables if so desired)

jtrmal · 2018-03-26T15:05:20Z

Thanks, Peter! y.

…

On Mon, Mar 26, 2018 at 11:03 AM, Peter Smit ***@***.***> wrote: Morfessor author here. We normally recommend installing through pip or conda, but in this case there is a much simpler way: - Clone the github - Set PYTHONPATH to include the repo - Set PATH to include the scripts directory Running setup.py has no benefit here in my opinion. Uninstalling is as simple as removing the repo (and cleaning up the variables if so desired) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2299 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKisXx8HR1DWmrjcaKnMTTb4I8KVW5WHks5tiQMtgaJpZM4S2Y5E> .

psmit · 2018-03-26T15:07:44Z

One issue, I noticed that the scripts in scripts/ are actually not having the executable bit set when cloning, I can fix that upstream (in a couple hours)

psmit · 2018-03-27T06:38:09Z

Script have executable bit set now: aalto-speech/morfessor@6c5ad6d

Changing the gist of the script to:

git clone https://github.com/aalto-speech/morfessor.git morfessor
(
    echo 'export MORFESSOR="$(pwd)/morfessor"'
    echo 'export PATH="$(pwd)/morfessor/scripts:$PATH"'
    echo 'export PYTHONPATH="$(pwd)/morfessor:$PYTHONPATH"'
) >> env.sh

should be enough.

psmit · 2018-03-27T07:51:58Z

I just tested this script and it works for me (Ubuntu 16.04)

jtrmal · 2018-03-28T18:19:54Z

Thanks to both of you! Merging.

* added install_morfessor.sh and its symbolic link * deleted symbolic link * retab with size 2 * simplified installation process acc. to psmit's advice

added install_morfessor.sh and its symbolic link

d3927d8

jtrmal reviewed Mar 22, 2018

View reviewed changes

lucas-jo mentioned this pull request Mar 22, 2018

[egs, script] Zeroth-Korean: Korean open-source corpus and its script #2296

Merged

jtrmal reviewed Mar 22, 2018

View reviewed changes

lucas-jo added 2 commits March 22, 2018 12:29

deleted symbolic link

b5d4a7d

retab with size 2

f44d724

jtrmal reviewed Mar 26, 2018

View reviewed changes

simplified installation process acc. to psmit's advice

626d2c5

Merge branch 'master' into install_morfessor

5d24cc0

jtrmal merged commit 5e6bd39 into kaldi-asr:master Mar 28, 2018

lucas-jo deleted the install_morfessor branch March 28, 2018 23:34

		@@ -0,0 +1 @@
		extras/install_morfessor.sh No newline at end of file

[tools, extras] morfessor installation script #2299

[tools, extras] morfessor installation script #2299

Uh oh!

Conversation

lucas-jo commented Mar 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucas-jo Mar 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danpovey commented Mar 24, 2018

Uh oh!

jtrmal commented Mar 26, 2018 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psmit commented Mar 26, 2018

Uh oh!

jtrmal commented Mar 26, 2018 via email

Uh oh!

psmit commented Mar 26, 2018

Uh oh!

psmit commented Mar 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psmit commented Mar 27, 2018

Uh oh!

jtrmal commented Mar 28, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lucas-jo Mar 22, 2018 •

edited

Loading

psmit commented Mar 27, 2018 •

edited

Loading