ablab / spades

SPAdes Genome Assembler
http://ablab.github.io/spades/
Other
749 stars 135 forks source link

Support Apple m1 #1062

Closed chenrui333 closed 4 months ago

chenrui333 commented 1 year ago

Is your feature request related to a problem? Please describe. For generic questions use Q&A section in the Discussions forum above.

Currently, the project does not build on Apple arm machines.

The failure log is below:

  In file included from /tmp/spades-20221226-6203-mtxvld/SPAdes-3.15.5/ext/src/ssw/ssw.c:38:
  In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.0/include/emmintrin.h:17:
  In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.0/include/xmmintrin.h:17:
  /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.0/include/mmintrin.h:14:2: error: "This header is only meant to be used on x86 and x64 architecture"
  #error "This header is only meant to be used on x86 and x64 architecture"
   ^
  /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.0/include/mmintrin.h:54:12: error: invalid conversion between vector type '__m64' (vector of 1 'long long' value) and integer type 'int' of different size
      return (__m64)__builtin_ia32_vec_init_v2si(__i, 0);
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

full error log is in here, https://github.com/Homebrew/homebrew-core/actions/runs/3780448890/jobs/6431013796

Additional context

related PR, https://github.com/Homebrew/homebrew-core/pull/119113

asl commented 1 year ago

Yes, this is a known problem. We are working on this, though it's a bit non-trivial given the amount of dependencies that are x86-only.

chenrui333 commented 1 year ago

Yes, this is a known problem. We are working on this, though it's a bit non-trivial given the amount of dependencies that are x86-only.

Cool, thanks for confirming it.

asl commented 1 year ago

Actually, we do have a port for several months as of now. However, we cannot test is reliably due to some internal infrastructure issues... :(

chenrui333 commented 1 year ago

happy to help, what are the things need to be tested out?

asl commented 1 year ago

happy to help, what are the things need to be tested out?

well, we're have a battery of internal tests on more than 100 different datasets / configurations :) So, it's a bit non-trivial to do externally

chenrui333 commented 1 year ago

I see 🤞

martin-g commented 11 months ago

Hi! Is there any progress on this task ? I see that many of the dependencies break on ARM64 (I test on Linux) but some of them have new releases which actually support ARM64, e.g. bwa and hmmer. For others it is easy to use sse2neon.h as a replacement for AVX. I have some progress at https://github.com/ablab/spades/compare/spades_3.15.5...martin-g:spades:linux-arm64-support but now I am stuck on https://github.com/ablab/spades/blob/3eed982f35e4021439b3b97012898505543afb14/assembler/ext/include/folly/SmallLocks.h#L46 https://github.com/facebook/folly/blob/main/folly/synchronization/SmallLocks.h has evolved a lot and I am not sure how to proceed.

asl commented 11 months ago

@martin-g The port was done. And it will be a part of next SPAdes release.

Note that you made output non-deterministic (see https://github.com/ablab/spades/compare/spades_3.15.5...martin-g:spades:linux-arm64-support#diff-ff9e3cc87fe5bc92da8881289b30ecd2205d0bc5a9b68428985699ee5e9169d5R266 for use of lrand48, etc.)

martin-g commented 11 months ago

And it will be a part of next SPAdes release.

Awesome! Do you have any approximate date ?

Note that you made output non-deterministic

Noted! First I was trying to fix the compilation errors and then I was going to review the changes more carefully.

bwesen commented 7 months ago

This would be cool, because on AWS for example the most price-efficient EC2 Linux machines are their own Graviton ARM64-based machines, that you can rent at spot pricing $0.1/hour for a 128 GB RAM, 16 vCPU machine, perfect for running memory intensive stuff like spades when needed.

asl commented 7 months ago

We just uploaded next branch that will be a base for next SPAdes releases. Please give it a try ;)

martin-g commented 7 months ago

I tried building it on Linux ARM64:

$ gcc --version
gcc (GCC) 10.3.1
$ g++ --version
g++ (GCC) 10.3.1
$ cmake -B build -S src -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
$ cmake --build build -j6

It failed with:

...
                 from /tmp/spades/spades/src/common/stages/construction.cpp:12:
/tmp/spades/spades/src/common/kmer_index/extension_index/inout_mask.hpp: In static member function ‘static constexpr char kmers::InOutMask::GetUnique(uint8_t)’:
/tmp/spades/spades/src/common/kmer_index/extension_index/inout_mask.hpp:69:49: error: narrowing conversion of ‘-1’ from ‘int’ to ‘char’ [-Wnarrowing]
   69 |                   3, -1, -1, -1, -1, -1, -1, -1 };
      |                                                 ^
...
asl commented 7 months ago

I tried building it on Linux ARM64:

This is outside the scope of the issue :) So far we do not support linux/aarch64 and did not test / port SPAdes there.

martin-g commented 7 months ago

Ah! It seems you are very close to support it though! :-) Cmake says that it built around 91% before the error! Any chance this error could be addressed somehow ? The Bioconda community would be very thankful !

asl commented 7 months ago

Cmake says that it built around 91% before the error!

Well, even 100% building does not mean it could run properly. But this is something we can try to support, though we certainly do not have resources for this.

martin-g commented 7 months ago

By resources you mean time+manpower or self-hosted Linux aarch64 runners ?

asl commented 7 months ago

By resources you mean time+manpower or self-hosted Linux aarch64 runners ?

Both. But mostly about man-power, as SPAdes team these days is just a company of volunteers devoting their spare time for this.

asl commented 4 months ago

Fixed with https://github.com/ablab/spades/releases/tag/v4.0.0