Skip to content
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

tools: remove openfst check #1531

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

KarelVesely84
Copy link
Contributor

@KarelVesely84 KarelVesely84 commented Apr 4, 2017

  • we will use the local open-fst, while we still can have the system-wide open-fst (not used by kaldi)
  • (note: this setup was previously forbidden)

* Now we will use the local open-fst, while we still can have the system-wide open-fst (this setup was previously forbidden).
@KarelVesely84
Copy link
Contributor Author

KarelVesely84 commented Apr 4, 2017

I better double-checked that this will cause no problems...
In `kaldi.mk' there are the absolute paths to the local OpenFST:

OPENFSTINC = /mnt/matylda5/iveselyk/DEVEL/kaldi-official/tools/openfst/include
OPENFSTLIBS = /mnt/matylda5/iveselyk/DEVEL/kaldi-official/tools/openfst/lib/libfst.so
OPENFSTLDFLAGS = -Wl,-rpath=/mnt/matylda5/iveselyk/DEVEL/kaldi-official/tools/openfst/lib

The headers are okay, as the '-I${OPENFSTINC}' option has priority over the system includes.
The library is also okay, as the LD_LIBRARY_PATH is stored directly to the binary with the '-rpath' option,
and we provied absolute path to the linker.

The related documentation is here:

Headers [4th paragraph]: https://gcc.gnu.org/onlinedocs/cpp/Search-Path.html#Search-Path
Library [section 3.4]: http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html

@danpovey
Copy link
Contributor

danpovey commented Apr 4, 2017 via email

@KarelVesely84
Copy link
Contributor Author

KarelVesely84 commented Apr 4, 2017

Hi apparently this is not true (It's from the GCC docs):

There are a number of command-line options you can use to add additional directories to the search path. The most commonly-used option is -Idir, which causes dir to be searched after the current directory (for the quote form of the directive) and ahead of the standard system directories. You can specify multiple -I options on the command line, in which case the directories are searched in left-to-right order.

The order for #include "fst/fstlib.h" is following:

  1. local directory [our kaldi src/subdir]
  2. -I dirs [our local openfst]
  3. system includes [the system installation of openfst]

@danpovey
Copy link
Contributor

danpovey commented Apr 4, 2017

hm, OK. Merging.

@danpovey danpovey merged commit 3c94401 into kaldi-asr:master Apr 4, 2017
@KarelVesely84
Copy link
Contributor Author

Thanks Dan!

kronos-cm added a commit to kronos-cm/kaldi that referenced this pull request Apr 5, 2017
* 'master' of https://github.com/kaldi-asr/kaldi:
  [src] Cosmetic change: remove 'train.tra' from usage messages (kaldi-asr#1529)
  [src] cudamatrix: speed up AddColSumMat with transfrom reduce kernel template (kaldi-asr#1530)
  [build]: remove openfst check (kaldi-asr#1531)
  [build,src,doc] Modify get_version.sh to deal better with whitespace (avoid space in version); minor fixes (kaldi-asr#1526)
  [scripts,egs] Adding options for using PCA instead of LDA+MLLT for ivectors used in ASR. Results are reported in the default TDNN recipe in AMI. Updating steps/online/nnet2/{train_diag_ubm.sh,train_ivector_extractor.sh} so that they now backup the contents of their destination directory if it already exists. (kaldi-asr#1514)
  [src] (minor) Added missing SetZero() to NaturalGradientAffineComponent::Scale() if scale==0.0 (kaldi-asr#1522)
  [src,doc] Fix several unrelated minor problems.  Thanks: gaoxinglong
  [src] Adding noexcept to hashing function objects (kaldi-asr#1519)
  [egs] Fix to egs/wsj/s5/run.sh (unset variable) (kaldi-asr#1517)
  [misc] remove eXecute permissions where not needed (kaldi-asr#1515)
  [src,scripts]: Several unrelated cosmetic changes
  [egs] fixes to babel pipeline; thanks to Fred Richardson (kaldi-asr#1509)
  [src] Fix exit code of extract-rows.cc (kaldi-asr#1510)
david-ryan-snyder pushed a commit to david-ryan-snyder/kaldi that referenced this pull request Apr 12, 2017
It appears there may be no good reason to disallow system-wide OpenFst.
@KarelVesely84 KarelVesely84 deleted the openfst_remove_check branch May 17, 2017 08:29
Skaiste pushed a commit to Skaiste/idlak that referenced this pull request Sep 26, 2018
It appears there may be no good reason to disallow system-wide OpenFst.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants