Re: [Kaldi-developers] OpenFST 1.4.1

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

Hi Dan

I’m attaching a new patch against kaldi-trunk. Please see my inline answers to your comments.

I also addressed the openfst symbolic linking problem in tools/Makefile that was mentioned in your other email.

I tested these changes on Ubuntu 12.04 with gcc-4.6 and clang-3.3. Clang build is printing a bunch of benign warnings on Linux (that we are suppressing in darwin makefiles with some extra flags). Maybe we should extract those flags from darwin makefiles to a common makefile shared on all builds. I mean I don’t see any reason why we should not fully support clang on Linux. Other than that, everything seems to be in working order on my end.

Cheers,
Dogan

> On Dec 9, 2014, at 1:53 PM, Daniel Povey <dp...@gm...> wrote:
> 
> Thanks..
> There are a few minor things I think are worth changing:
> 
> -  patches should now go not in tools/ but in tools/extras/.

Done. I also moved the sequitur.patch into tools/extras and changed its build script to use this new location. 

> 
> - I'd prefer it if the test for the OpenFST version and the c++0x flag
> were in the configure script rather than in the makefiles/* (if some
> setups need extra flags you can put a test in the corresponding
> makefile though).  This will avoid the need to edit all the
> makefiles/*, and also I prefer kaldi.mk to just have a few simple
> options in it rather than having all kinds of conditional statements.
> Then it's easier to hand-edit if the user feels like it.  I'm OK with
> things breaking if someone re-installs OpenFst without rerunning
> "configure", because the concept is that you rerun "configure" any
> time you change something bug.

Done.

> 
> -in util/stl-utils.h, there are redundant #ifdef/#elif blocks, I think
> it would be easier to just do
> #if defined(_MSC_VER) || (__cplusplus > 199711L)
> #include <unordered_map>
> #include <unordered_set>
> using std::unordered_map;
> using std::unordered_set;
> #else
> …
> 

It seems the support for these macros is quite a mess at the moment. For instance I just noticed that the __cplusplus macro is defined to be 1 in gcc-4.6 (default on Ubuntu 12.04) when c++11 support is enabled. I had to change that condition to the following one to get things compile on Ubuntu.

if __cplusplus > 199711L || defined(__GXX_EXPERIMENTAL_CXX0X__)

I left redundant #ifdef blocks in place in case we need to further adjust these checks in the future. Once these things are somewhat settled we can get rid of the redundant cases. What do you think?

> - Wouldn't it be less confusing to add the -std=c++11 option instead
> of the -std=c++0x option?
> 

-std=c++11 is not supported on gcc-4.6.

> - For me, on Linux, the kaldi.mk did not work out of the box because
> your statement adding the -std=c++0x option was above the statement
> 
> CXXFLAGS = -msse -msse2 -Wall -I.. \
>       -pthread \
> [etc.]
> 
> Presumably when you put this test in the configure script instead of
> kaldi.mk, you can add this at the end rather than the beginning of
> kaldi.mk.
> 

I fixed this on Ubuntu 12.04 by adding the necessary c++11 flags to EXTRA_CXXFLAGS. Could you check if your Linux distro is happy after this change?

> I am not able to test this on my Mac (OS X 10.7.5).  I get this:
> configure: error: in `/Users/danielpovey/kaldi/trunk/tools/openfst-1.4.1':
> configure: error: C++ compiler cannot create executables
> See `config.log' for more details
> make: *** [openfst-1.4.1/Makefile] Error 77
> and openfst/config.log has nothing interesting.  I am guessing it
> means my C++ compiler is out of date.  I checked MacPorts and there is
> nothing newer available.  [my version is "gcc version 4.2.1 (Based on
> Apple Inc. build 5658) (LLVM build 2336.1.00)”]

You need gcc >= 4.6 for c++11 support. You can install the latest gcc (as well as older ones if needed) with homebrew <http://brew.sh/>.

> Anyway, this probably doesn't matter much.  It just means you should
> show me what the error was in that failing test so I will know how to
> fix it.

I’m attaching an output log.

> 
> Dan
> 
> On Tue, Dec 9, 2014 at 4:36 AM, Dogan Can <dog...@us...> wrote:
>> Hi Dan
>> 
>> Sure, I’m attaching a patch against the kaldi-trunk. I just made a bunch of additional changes to get the failing tests succeed. At the moment, the only test failing on Mac OS 10.10 is sgmm/estimate-am-sgmm-multi-test. Could you take a look at that? I think it needs a threshold adjustment or something like that.
>> 
>> Cheers,
>> Dogan
>> 
>> -------------------------------------
>> 
>> Installation instructions:
>> 
>> cd tools
>> make -j 8 OPENFST_VERSION=1.4.1
>> cd ../src
>> ./configure
>> make -j 8 depend
>> make -j 8
>> 
>> If OPENFST_VERSION is not set during the make call, then it defaults to 1.3.2, i.e. current installation instructions remain as they are.
>> 
>> Apart from the script changes and a bunch of conditional includes, the major change in this patch is the addition of "util/basic-filebuf.h” which defines a modified version of the std::basic_filebuf class from libc++. It allows one to create a basic_filebuf object from an existing FILE* handle or file descriptor when linking against libc++. This is a fairly large change and has the potential to cause some regressions.
>> 
>> By the way, “make -j 8 depend” step produces a bunch of “subframework" warnings on Mac OS 10.10 when compiling with GCC. It seems Apple dropped support for some previously deprecated stuff and GCC hasn’t caught up yet.
>> 
>> Also note that this patch includes a few minor/cosmetic fixes here and there.
>> 
>> 
>> 
>> 
>>> On Dec 8, 2014, at 10:39 AM, Daniel Povey <dp...@gm...> wrote:
>>> 
>>> Dogan, maybe you could attach your patch here and let Simon test it?
>>> Dan
>>> 
>>> 
>>> On Mon, Dec 8, 2014 at 1:38 PM, Dogan Can <dog...@us...> wrote:
>>>> Hi Dan
>>>> 
>>>> It builds on Mac OS X 10.10 both with g++ and clang. I haven't done any
>>>> regression tests though. Also I haven't tested it on Linux yet.
>>>> 
>>>> Paul, did you get a chance to test it on your end?
>>>> 
>>>> Cheers
>>>> Dogan
>>>> 
>>>> —
>>>> Sent from Mailbox
>>>> 
>>>> 
>>>> On Mon, Dec 8, 2014 at 10:27 AM, Daniel Povey <dp...@gm...> wrote:
>>>>> 
>>>>> Paul Dixon has already done some work to get Kaldi to compile with
>>>>> OpenFst 1.4 (and C++11), but it is not committed yet. Dogan was going
>>>>> to help get it committed, but I don't know how far along this is.
>>>>> Dogan, any comment?
>>>>> Dan
>>>>> 
>>>>> 
>>>>> On Mon, Dec 8, 2014 at 12:19 PM, Jan Trmal <af...@ce...> wrote:
>>>>>> The OpenFST 1.4.1 needs latest C++ standard (C++11). I'm not sure which
>>>>>> of
>>>>>> the C++11 features it needs, but not all compilers support that standard
>>>>>> fully, or you have to use relatively recent version. That might be a bit
>>>>>> of
>>>>>> inconvenience for some people. Feel free to to experiment with it in
>>>>>> connection to Kaldi -- there is not specific reason why to avoid it.
>>>>>> y.
>>>>>> 
>>>>>> On Mon, Dec 8, 2014 at 8:47 AM, Simon Klüpfel <sim...@gm...>
>>>>>> wrote:
>>>>>>> 
>>>>>>> Hi,
>>>>>>> 
>>>>>>> I wondered if there exists any version of Kaldi using OpenFST version
>>>>>>> 1.4.1 by default, or if there would be any interest in having this
>>>>>>> option or any reason not to use it?!
>>>>>>> 
>>>>>>> I am using Thrax for a rule based grammar used as the language model,
>>>>>>> and the current version requires the latest version of OpenFST.
>>>>>>> 
>>>>>>> Is there any interest in reviewing, if I should add this as an option
>>>>>>> to
>>>>>>> the installation of the tools? I.e., adding support for (a patched)
>>>>>>> 1.4.1 version, as well as options for the OpenGrm tools (Thrax and
>>>>>>> perhaps as well NGram Library, if anyone might want to try it out)?
>>>>>>> 
>>>>>>> All the best,
>>>>>>> 
>>>>>>> Simon
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> ------------------------------------------------------------------------------
>>>>>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>>>>>> from Actuate! Instantly Supercharge Your Business Reports and
>>>>>>> Dashboards
>>>>>>> with Interactivity, Sharing, Native Excel Exports, App Integration &
>>>>>>> more
>>>>>>> Get technology previously reserved for billion-dollar corporations,
>>>>>>> FREE
>>>>>>> 
>>>>>>> 
>>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
>>>>>>> _______________________________________________
>>>>>>> Kaldi-developers mailing list
>>>>>>> Kal...@li...
>>>>>>> https://lists.sourceforge.net/lists/listinfo/kaldi-developers
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> ------------------------------------------------------------------------------
>>>>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>>>>> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
>>>>>> with Interactivity, Sharing, Native Excel Exports, App Integration &
>>>>>> more
>>>>>> Get technology previously reserved for billion-dollar corporations, FREE
>>>>>> 
>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
>>>>>> _______________________________________________
>>>>>> Kaldi-developers mailing list
>>>>>> Kal...@li...
>>>>>> https://lists.sourceforge.net/lists/listinfo/kaldi-developers
>>>>>> 
>>>> 
>>>> 
>> 
>> 