ddarriba / modeltest

Best-fit model selection

GNU General Public License v3.0

72 stars 21 forks source link

Replace non-portable function `strcasecmp()`? #40

Open ms609 opened 3 years ago

ms609 commented 3 years ago

In meta.cpp and utils.cpp, the function strcasecmp is non-portable. This prevents me from building modeltest on Windows. Would it be possible to use an alternative -- for example, converting optarg to lowercase in meta.cpp, or converting tmpchar to uppercase in utils.cpp?

xflouris commented 3 years ago

Hi @ms609 , I assume you're using the Microsoft compiler (MSVC). If that's the case, a proper to build under windows is to replace strcasecmp() with _stricmp(). You can add the following at the top of utils.cpp or in a header file that is included:

ifdef _MSC_VER

define strncasecmp _strnicmp

define strcasecmp _stricmp

endif

ms609 commented 3 years ago

Hi – I was using MinGW / MSYS2 (as suggested on the README). Everything seems to work without modifying case...

amkozlov commented 3 years ago

Hi @ms609, have you by any chance benchmarked the performance of your Windows binary? I tried to compile raxml-ng with MSYS2 today, and it worked BUT was 20x slower than on Linux :-O So I'm wondering whether there is something wrong with my Windows installation or build setup, or that's the price one has to pay for emulating Linux/POSIX environment under Windows?

ms609 commented 3 years ago

Sorry, I've not benchmarked the performance, and this isn't something I'd know how to do... but if you can point me to some suitable instructions, I'd be happy to do so, in case that helps to pin down the source of the sluggishness?

amkozlov commented 3 years ago

@ms609 it seems like multi-threading is (part of the) problem, could you please compare modeltest runtime with one vs. multiple threads? Also, if you have access to a dual-boot system, it would be interesting to see Windows vs. Linux runtime on the same dataset. Thanks!