Re: [Kaldi-developers] OpenFST 1.4.1

[Daniel P. <dp...@gm...>]

OK, I tested your patch and it seems to work for the current OpenFst.

I noticed another issue-- you seem to be using openfst-1.3.2 as the
"older" version, but the current Kaldi uses openfst-1.3.4.
This may be why you have to use that new patch to fix begin_->begin
and end_->end in IntervalSet.  I think perhaps you should update your
setup to use openfst-1.3.4 as the older supported version.

I will check in a fix for the sgmm-*-multi-test separately.

Something else is that it's not very nice to have to specify
OPENFST_VERSION=1.4.2 on the command line to 'make'.  If someone were
to do this, and later type "make" again, it would go back to the older
version.  I think it might be better to instruct people to uncomment a
line in the Makefile if they want this.  Any comment?  Adding a
'configure' script at this level seems too heavyweight for now.

Dan


On Tue, Dec 9, 2014 at 10:33 PM, Dogan Can <dog...@us...> wrote:
> 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.
>
> 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
>
>
>
>
>
>
>