rcedgar / muscle

Multiple sequence and structure alignment with top benchmark scores scalable to thousands of sequences. Generates replicate alignments, enabling assessment of downstream analyses such as trees and predicted structures.
https://drive5.com/muscle
GNU General Public License v3.0
188 stars 22 forks source link

-std=c++11 CFLAG needed in Makefile #4

Closed joelb123 closed 2 years ago

joelb123 commented 3 years ago

Make fails on recent gcc (>8, I think) unless this CFLAG is added

mschubert commented 2 years ago

Confirmed for gcc=11.1.0 on Arch

nsoranzo commented 2 years ago

-std=c++14 works as well.

With -std=c++17 the main errors are:

g++ -fvisibility-inlines-hidden -std=c++17 -fopenmp -ffast-math -msse -mfpmath=sse -O3 -DNDEBUG -c -o o/addconfseq.o addconfseq.cpp
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:225:1: error: reference to ‘byte’ is ambiguous
  225 | byte *ReadAllStdioFile32(FILE *f, uint32 &FileSize);
      | ^~~~
In file included from /usr/include/c++/9/bits/stl_algobase.h:61,
                 from /usr/include/c++/9/bits/char_traits.h:39,
                 from /usr/include/c++/9/string:40,
                 from myutils.h:22,
                 from muscle.h:21,
                 from addconfseq.cpp:1:
/usr/include/c++/9/bits/cpp_type_traits.h:404:30: note: candidates are: ‘enum class std::byte’
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:77:23: note:                 ‘typedef unsigned char byte’
   77 | typedef unsigned char byte;
      |                       ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:450:29: error: ‘bool myislower(char)’ redeclared as different kind of entity
  450 | inline bool myislower(char c) { return (c & 0x20) != 0; }
      |                             ^
myutils.h:449:13: note: previous declaration ‘bool myislower’
  449 | inline bool myislower(byte c) { return (c & 0x20) != 0; }
      |             ^~~~~~~~~
In file included from multisequence.h:4,
                 from muscle.h:23,
                 from addconfseq.cpp:1:
In file included from muscle.h:25,
                 from addconfseq.cpp:1:
mysparsemx.h: In constructor ‘MySparseMx::MySparseMx()’:
mysparsemx.h:30:3: error: ‘m_ValueVec’ was not declared in this scope
   30 |   m_ValueVec = 0;
      |   ^~~~~~~~~~
mysparsemx.h:35:3: error: ‘m_X’ was not declared in this scope; did you mean ‘m_LX’?
   35 |   m_X = 0;
      |   ^~~
      |   m_LX
mysparsemx.h:36:3: error: ‘m_Y’ was not declared in this scope; did you mean ‘m_LY’?
   36 |   m_Y = 0;
      |   ^~~
      |   m_LY
rcedgar commented 2 years ago

Hi guys -- give me a day or two here please. I forgot about a pull request on https://github.com/rcedgar/vcxproj2makefile; this is the script which generates the Makefile, not VSCode. FWIW the -std=c++11 is not needed through at least gcc v9, which appears to be the default version on Ubuntu 20. But it seems to do no harm so I'm probably ok adding it.

It seems unrealistic to expect that the Makefile is compatible with every possible configuration out there without using heavyweight tools like autoconf that I simply don't have time to figure out and implement. IMO the pragmatic solution is for third parties to make small hacks as needed; I don't have the resources or expertise to make a repository that integrates with automated updates in other toolsets.

colinbrislawn commented 2 years ago

IMO the pragmatic solution is for third parties to make small hacks as needed

This is exactly what FastTree2 does. They publish the source and install guide here, and the bioconda community independently set up the bioconda-recipe. That worked out well: bioconda downloads

compatible with every possible configuration... a repository that integrates with automated updates in other toolsets

And that's why god created Deployment Engineers.

Thanks for helping us sort this out. No rush!

rcedgar commented 2 years ago

So I finally found time to update https://github.com/rcedgar/vcxproj2makefile, anything more I should do here @mschubert @colinbrislawn @nsoranzo?

V-Z commented 2 years ago

I get same errors with gcc 11.2 on openSUSE Linux. I try to add -std=c++11, but get same errors...

V-Z commented 2 years ago

I tried the vcxproj2makefile.py (sorry, I haven't realized yesterday I could try it) and when running make I get Makefile:4: *** Recursive variable 'CXXFLAGS' references itself (eventually). Stop.... As Linux user (and occasional packager), one standard and universal Makefile would be brilliant. :-)

rcedgar commented 2 years ago

"As Linux user (and occasional packager), one standard and universal Makefile would be brilliant. :-)" Absolutely, and I would love to make such Makefiles. I'm far from an expert in this area, but I believe this would be a lot of work to implement because of variations in compilers, system libraries, header files etc. There are tools such as autoconf to do this, but I don't have the bandwidth to learn them. If anyone has suggestions for simpler hacks to improve portability between gcc versions and o/s releases I'd be glad to look into it.

mschubert commented 2 years ago

Why not provide a standard Makefile with the project here? (like the one that @joelb123 proposed or @nsoranzo sent as a PR)

This should work on (almost) all systems, not need external dependencies (like python and the makefile-generating script), no need for autconf, not needed to be changed after, and only be ~ 10 LOC.

The simplest one would probably be (variables explained here):

CXXFLAGS = -std=c++11 -O3 -DNDEBUG -fopenmp -ffast-math -msse -mfpmath=sse
LDFLAGS = -O3 -fopenmp -pthread -lpthread

CPP = $(wildcard *.cpp)

muscle5: $(CPP:%.cpp=%.o)
    $(CXX) $(LDFLAGS) $^ -o $@

%.o: %.cpp $(wildcard *.h)
    $(CXX) $(CXXFLAGS) -c -o $@ $<
rcedgar commented 2 years ago

The problem is explained in the comments to that PR

rcedgar commented 2 years ago

In more detail, I develop in MS Visual C++ which has its own project format, there is not a 1:1 between source files in the project directory and source files which should be included in the build. This is why I did the vcxproj2makefile.py hack -- this extracts the active source files and generates a simple Makefile where each source has its own build line.

rcedgar commented 2 years ago

Yeah this whole makefile/gcc/vcxproj2makefile stuff is clearly not working well enough to be a practical solution, I'm going to spend some time trying to engineer something better ... wish me luck :-)

joelb123 commented 2 years ago

It's really difficult to solve problems collaboratively when there are hidden constraints that cause well-meaning PRs to get rejected. This costs everybody time. For Muscle V4 I got a factor of ~2 performance increase by doing program-guided optimization, which I'd like to contribute for V5, but I can't do that while the Makefile situation is like it is.

I'm not a VS user, but ton of people (most users, I'd guess) use VS with makefiles. Here are instructions on how to do that.

I do sometimes have private files associated with an otherwise-public repository corresponding to features I'm not yet ready to release. Git makes it easy to place different branches in different repositories and one can easily be private.

rcedgar commented 2 years ago

@joelb123 thanks for the feedback, I'm also frustrated because I take pride in engineering low-friction software e.g. by avoiding dependencies on third-party libraries which create so many problems in scientific applications. Collaborating in an open source framework is new to me, for whatever reason with MUSCLE and my other previous software this kind of thing never came up, I'm not sure why the difference. I'm trying to learn and welcome constructive suggestions.

nsoranzo commented 2 years ago

I tried the vcxproj2makefile.py (sorry, I haven't realized yesterday I could try it) and when running make I get Makefile:4: *** Recursive variable 'CXXFLAGS' references itself (eventually). Stop.

@V-Z Sorry, my fault, will be fixed by https://github.com/rcedgar/vcxproj2makefile/pull/3

As Linux user (and occasional packager), one standard and universal Makefile would be brilliant. :-)

A single Makefile will be generated by https://github.com/rcedgar/vcxproj2makefile now that https://github.com/rcedgar/vcxproj2makefile/pull/1 has been merged.

So I finally found time to update https://github.com/rcedgar/vcxproj2makefile, anything more I should do here?

@rcedgar Fixing the C++17 small issues in https://github.com/rcedgar/muscle/issues/4#issuecomment-972192599 would be great, otherwise we should temporarily add -std=c++14 to the CXXFLAGS in the Makefile.

V-Z commented 2 years ago

@nsoranzo Thank You, I now applied vcxproj2makefile.py and I get errors:

$ make
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -c -o o/addconfseq.o addconfseq.cpp
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:221:1: error: reference to ‘byte’ is ambiguous
  221 | byte *ReadAllStdioFile32(FILE *f, uint32 &FileSize);
      | ^~~~
In file included from /usr/include/c++/11/bits/stl_algobase.h:61,
                 from /usr/include/c++/11/bits/char_traits.h:39,
                 from /usr/include/c++/11/string:40,
                 from myutils.h:22,
                 from muscle.h:21,
                 from addconfseq.cpp:1:
/usr/include/c++/11/bits/cpp_type_traits.h:404:30: note: candidates are: ‘enum class std::byte’
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:73:23: note:                 ‘typedef unsigned char byte’
   73 | typedef unsigned char byte;
      |                       ^~~~
myutils.h:222:1: error: reference to ‘byte’ is ambiguous
  222 | byte *ReadAllStdioFile64(FILE *f, uint64 &FileSize);
      | ^~~~
In file included from /usr/include/c++/11/bits/stl_algobase.h:61,
                 from /usr/include/c++/11/bits/char_traits.h:39,
                 from /usr/include/c++/11/string:40,
                 from myutils.h:22,
                 from muscle.h:21,
                 from addconfseq.cpp:1:
/usr/include/c++/11/bits/cpp_type_traits.h:404:30: note: candidates are: ‘enum class std::byte’
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:73:23: note:                 ‘typedef unsigned char byte’
   73 | typedef unsigned char byte;
      |                       ^~~~
myutils.h:224:1: error: reference to ‘byte’ is ambiguous
  224 | byte *ReadAllStdioFile(FILE *f, uint32 &FileSize);
      | ^~~~
In file included from /usr/include/c++/11/bits/stl_algobase.h:61,
                 from /usr/include/c++/11/bits/char_traits.h:39,
                 from /usr/include/c++/11/string:40,
                 from myutils.h:22,
                 from muscle.h:21,
                 from addconfseq.cpp:1:
/usr/include/c++/11/bits/cpp_type_traits.h:404:30: note: candidates are: ‘enum class std::byte’
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:73:23: note:                 ‘typedef unsigned char byte’
   73 | typedef unsigned char byte;
      |                       ^~~~
myutils.h:225:1: error: reference to ‘byte’ is ambiguous
  225 | byte *ReadAllStdioFile64(FILE *f, uint64 &FileSize);
      | ^~~~
In file included from /usr/include/c++/11/bits/stl_algobase.h:61,
                 from /usr/include/c++/11/bits/char_traits.h:39,
                 from /usr/include/c++/11/string:40,
                 from myutils.h:22,
                 from muscle.h:21,
                 from addconfseq.cpp:1:
/usr/include/c++/11/bits/cpp_type_traits.h:404:30: note: candidates are: ‘enum class std::byte’
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:73:23: note:                 ‘typedef unsigned char byte’
   73 | typedef unsigned char byte;
      |                       ^~~~
myutils.h:227:1: error: reference to ‘byte’ is ambiguous
  227 | byte *ReadAllStdioFile32(const string &FileName, uint32 &FileSize);
      | ^~~~
In file included from /usr/include/c++/11/bits/stl_algobase.h:61,
                 from /usr/include/c++/11/bits/char_traits.h:39,
                 from /usr/include/c++/11/string:40,
                 from myutils.h:22,
                 from muscle.h:21,
                 from addconfseq.cpp:1:
/usr/include/c++/11/bits/cpp_type_traits.h:404:30: note: candidates are: ‘enum class std::byte’
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:73:23: note:                 ‘typedef unsigned char byte’
   73 | typedef unsigned char byte;
      |                       ^~~~
myutils.h:228:1: error: reference to ‘byte’ is ambiguous
  228 | byte *ReadAllStdioFile64(const string &FileName, uint64 &FileSize);
      | ^~~~
In file included from /usr/include/c++/11/bits/stl_algobase.h:61,
                 from /usr/include/c++/11/bits/char_traits.h:39,
                 from /usr/include/c++/11/string:40,
                 from myutils.h:22,
                 from muscle.h:21,
                 from addconfseq.cpp:1:
/usr/include/c++/11/bits/cpp_type_traits.h:404:30: note: candidates are: ‘enum class std::byte’
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from muscle.h:21,
                 from addconfseq.cpp:1:
myutils.h:73:23: note:                 ‘typedef unsigned char byte’

and so on... So I'm not sure if I need to do anymore modifications in the Makefile...?

nsoranzo commented 2 years ago

@V-Z These are the same of https://github.com/rcedgar/muscle/issues/4#issuecomment-972192599 . As I mentioned in my latest reply, these should be fixed to support C++17, but you can temporarily workaround them by adding -std=c++14 to the CXXFLAGS in the Makefile.

V-Z commented 2 years ago

Ah, sorry, my fault. :-( OK, did so and I get:

$ make
mkdir -p o/
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/addconfseq.o addconfseq.cpp
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/align.o align.cpp
...
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/msa.o msa.cpp
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/msa2.o msa2.cpp
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/myutils.o myutils.cpp
myutils.cpp:2058:10: fatal error: ../ver/counter.h: No such file or directory
 2058 | #include "../ver/counter.h"
      |          ^~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:188: o/myutils.o] Error 1

Well, it's really missing... but... where to get it as it's not part of the repository...? At least there is progress. :-)

nsoranzo commented 2 years ago

That error has already been fixed in the main branch some time ago, if you search in the closed issues.

V-Z commented 2 years ago

OK, working with the released version... So... after cloning the git and adding -std=c++14 I endup with:

$ make
mkdir -p o/
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/addconfseq.o addconfseq.cpp
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/align.o align.cpp
...
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/usage.o usage.cpp
g++  -DNDEBUG -pthread  -O3 -fopenmp -ffast-math -msse -mfpmath=sse -std=c++14 -c -o o/usorter.o usorter.cpp
g++  -O3 -fopenmp -pthread -lpthread -static o/addconfseq.o o/align.o o/alignpairflat.o o/allocflat.o o/alnalnsflat.o o/alnmsasflat.o o/alnmsasflat3.o o/alpha.o o/alpha3.o o/assertsameseqs.o o/calcalnscoresparse.o o/heatmapcolors.o o/jalview.o o/letterconf.o o/diagbox.o o/buildposterior3flat.o o/buildpostflat.o o/bwdflat3.o o/calcalnflat.o o/calcalnscoreflat.o o/calcposteriorflat.o o/conspairflat.o o/consflat.o o/defaulthmmparams.o o/derep.o o/disperse.o o/dividetree.o o/eacluster.o o/efabestcols.o o/efabestconf.o o/efaexplode.o o/efastats.o o/ensemble.o o/fa2efa.o o/fwdflat3.o o/getpairs.o o/getpostpairsalignedflat.o o/help.o o/letterconfhtml.o o/logdistmx.o o/logmx.o o/maxcc.o o/mpcflat.o o/eadistmx.o o/eadistmxmsas.o o/fasta.o o/fasta2.o o/mysparsemx.o o/perturbhmm.o o/getconsseq.o o/globalinputms.o o/guidetreejoinorder.o o/hmmdump.o o/hmmparams.o o/jointrees.o o/logaln.o o/main.o o/msastats.o o/multisequence.o o/permutetree.o o/pprog.o o/pprog2.o o/pprogt.o o/probcons.o o/msa.o o/msa2.o o/myutils.o o/colscoreefa.o o/qscoreefa.o o/randomchaintree.o o/relabel.o o/resample.o o/stripgappyrows.o o/tree.o o/tree2.o o/tree4.o o/treefromfile.o o/treetofile.o o/pairhmm.o o/progalnflat.o o/project.o o/qscore.o o/qscore2.o o/qscorer.o o/quarts.o o/refineflat.o o/relaxflat.o o/seb8.o o/sequence.o o/setprobconsparams.o o/seq.o o/stripgappycols.o o/super4.o o/super5.o o/testfb.o o/testlog.o o/testscoretype.o o/textfile.o o/totalprobflat.o o/tracebackflat.o o/transaln.o o/transq.o o/treeperm.o o/treesplitter.o o/treesubsetnodes.o o/trimtoref.o o/trimtorefefa.o o/uclust.o o/upgma5.o o/usage.o o/usorter.o  -o o/muscle5
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-linux/bin/ld: cannot find -lm
/usr/lib64/gcc/x86_64-suse-linux/11/../../../../x86_64-suse-linux/bin/ld: cannot find -lc
collect2: error: ld returned 1 exit status
make: *** [Makefile:182: o/muscle5] Error 1

I hope now it's not my fault that I'd missed something again....

rcedgar commented 2 years ago

Please see PR #17 for my attempt to resolve these issues. I'll wait a couple of weeks for comments before merging.

rcedgar commented 2 years ago

Fixed in v5.1.