Pre-compiled CLIXO does not work

[cthoyt]

Having pre-compiled C++ code in the repository is a bit confusing, since it doesn't generally work on each person's system. This isn't obvious from using the python code, since using subprocess.Popen fails completely silently if the current system can't run the clixo binary. Note: I'm running Mac OS 11.13 Sierra with the following clang:

$ g++ --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 10.0.0 (clang-1000.11.45.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Further, I went to re-compile myself and got the following error due to the inclusion of the -static flag to include static libraries:

$ make clean
rm *.o
(ddot) [529] [13:20] [cthoyt@wlan-224:~/dev/ddot/ddot/mhk7-clixo_0.3-cec3674]
$ make all
g++ -Wall -O4 -c -std=c++11 -fomit-frame-pointer -funroll-loops -fforce-addr -fexpensive-optimizations -static -I . clixo.cpp
clang: warning: -O4 is equivalent to -O3 [-Wdeprecated]
clang: warning: optimization flag '-fexpensive-optimizations' is not supported [-Wignored-optimization-argument]
In file included from clixo.cpp:3:
./dagConstruct.h:850:32: warning: '/*' within block comment [-Wcomment]
    /*if ((printClusterInfo && /*!combiningNow &&*//* !currentClusters[clusterToDelete].wasCheckedFinal()) || (currentClusters[clusterToDelete]...
                               ^
./dagConstruct.h:932:14: warning: '/*' within block comment [-Wcomment]
      /*if ((/*!combiningNow && *//*!clusterToExtend_it->wasCheckedFinal()) || clusterToExtend_it->isValid()) {
             ^
./dagConstruct.h:1403:17: warning: private field 'numClusters' is not used [-Wunused-private-field]
  unsigned long numClusters;
                ^
3 warnings generated.
g++ -Wall -O4 -std=c++11 -fomit-frame-pointer -funroll-loops -fforce-addr -fexpensive-optimizations -static -I . clixo.o -o clixo
ld: library not found for -lcrt0.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [clixo] Error 1

I'm not much of a dev-ops person and I'm still working through the issue, but this thread had some information that sort of worked: https://discussions.apple.com/thread/1945589?answerId=9213715022#9213715022. It suggested removing the -static flags, or even better, adding more conditionals in the makefile to check what system is being run and if this is a good idea or not. After, I was able to re-compile it and got the following output:

$ make clean
rm *.o
(ddot) [533] [13:23] [cthoyt@wlan-224:~/dev/ddot/ddot/mhk7-clixo_0.3-cec3674]
$ make all
g++ -Wall -O4 -c -std=c++11 -fomit-frame-pointer -funroll-loops -fforce-addr -fexpensive-optimizations -I . clixo.cpp
clang: warning: -O4 is equivalent to -O3 [-Wdeprecated]
clang: warning: optimization flag '-fexpensive-optimizations' is not supported [-Wignored-optimization-argument]
In file included from clixo.cpp:3:
./dagConstruct.h:850:32: warning: '/*' within block comment [-Wcomment]
    /*if ((printClusterInfo && /*!combiningNow &&*//* !currentClusters[clusterToDelete].wasCheckedFinal()) || (currentClusters[clusterToDelete]...
                               ^
./dagConstruct.h:932:14: warning: '/*' within block comment [-Wcomment]
      /*if ((/*!combiningNow && *//*!clusterToExtend_it->wasCheckedFinal()) || clusterToExtend_it->isValid()) {
             ^
./dagConstruct.h:1403:17: warning: private field 'numClusters' is not used [-Wunused-private-field]
  unsigned long numClusters;
                ^
3 warnings generated.
g++ -Wall -O4 -std=c++11 -fomit-frame-pointer -funroll-loops -fforce-addr -fexpensive-optimizations  -I . clixo.o -o clixo
g++ -Wall -O4 -c -std=c++11 -fomit-frame-pointer -funroll-loops -fforce-addr -fexpensive-optimizations -I . clustersToDAG.cpp
clang: warning: -O4 is equivalent to -O3 [-Wdeprecated]
clang: warning: optimization flag '-fexpensive-optimizations' is not supported [-Wignored-optimization-argument]
In file included from clustersToDAG.cpp:3:
./dagConstruct.h:850:32: warning: '/*' within block comment [-Wcomment]
    /*if ((printClusterInfo && /*!combiningNow &&*//* !currentClusters[clusterToDelete].wasCheckedFinal()) || (currentClusters[clusterToDelete]...
                               ^
./dagConstruct.h:932:14: warning: '/*' within block comment [-Wcomment]
      /*if ((/*!combiningNow && *//*!clusterToExtend_it->wasCheckedFinal()) || clusterToExtend_it->isValid()) {
             ^
./dagConstruct.h:1403:17: warning: private field 'numClusters' is not used [-Wunused-private-field]
  unsigned long numClusters;
                ^
3 warnings generated.
g++ -Wall -O4 -std=c++11 -fomit-frame-pointer -funroll-loops -fforce-addr -fexpensive-optimizations  -I . clustersToDAG.o -o clustersToDAG

[michaelkyu]

Thanks for this investigation. I agree with the removal of the -static flag. If you can create a pull request for just your changes to the makefiles, I'll happily accept them, but I'm not sure if you can isolate pull requests to specific files. If this is too complicated, I can just make the same edits to the makefiles in the master branch of ddot.

I'll also remove the precompiled binaries.

The CliXO code in the ddot/mhk7-clixo_0.3-cec3674/ directory is actually copied from the original CliXO repo: https://github.com/mhk7/clixo_0.3. Ideally, I want to remove this code from the ddot repo and just have ddot download the CliXO code during packaging and setup. Admittedly, I've spent a long time trying to understand how to create a Python package with C++ code, but I still haven't figured out a good solution. If you have experience in this, I'd be interested in your suggestions.

[cthoyt]

One solution might be to ask users before installing ddot that they've installed clixo and alignOntology and to make sure that the executables are on the PATH so they can be called without trying to introspect too much about where they are.

Then it would just be a matter of adding to the installation documentation how to take care of downloading/compiling the C++ code and adding it.