ablab / spades

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

Change GetUnique() return type from char to init8_t #1269

Closed martin-g closed 5 months ago

martin-g commented 6 months ago

char is unsigned on aarch64

Related to: https://github.com/ablab/spades/issues/1062#issuecomment-2019920641

With this change I was able to fully build SPAdes on Linux ARM64:

mgrigorov in 🌐 euler-arm-22 in spades on  next 
❯ file build/bin/*
build/bin/spades-bwa:                  ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=562164721589d0b609cd56ac36c3186706248059, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-convert-bin-to-fasta: ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=74eda85d3b74b60020a4cd6657de655f3396fcde, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-core:                 ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=b5e24e4001f549108cab10b9745debf2da913be2, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-corrector-core:       ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=ead8c8c5f9a451a02bbb76b6841dc55f38b51fb5, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-gbuilder:             ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=b76e897b78e923d5cdda9c95463810bb42c40dfc, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-gfa-split:            ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=cae67bbe81c35d566da04826af6a04a85546d9fb, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-gmapper:              ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=5b8a3c6077b43c38e41d06c6d8aadc2ecbde9fcc, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-gsimplifier:          ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=7f0b67d6449d6fa4253d429e520358995c4e4f60, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-hammer:               ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=4368c59963ee313387a09cd00033531cbcc5e8e1, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-ionhammer:            ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=432aaf9b887ff8521ee073510fe47ffc379fff1b, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-kmer-estimating:      ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=ef045fa9bee7c8f061d2e133e584bebfaca731f3, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-kmercount:            ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=2f94a2b137de314fe0a12ccf4fbe47ee52afb5a0, for GNU/Linux 3.7.0, with debug_info, not stripped
build/bin/spades-read-filter:          ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, BuildID[sha1]=a7ebfa2a63cbebff0d29f490d570b5581e72b515, for GNU/Linux 3.7.0, with debug_info, not stripped

mgrigorov in 🌐 euler-arm-22 in spades on  next 
❯ ./build/bin/spades-bwa 

Program: bwa (alignment via Burrows-Wheeler transformation)
Version: 0.7.16a-r1185-dirty
Contact: Heng Li <lh3@sanger.ac.uk>

Usage:   bwa <command> [options]

Command: index         index sequences in the FASTA format
         mem           BWA-MEM algorithm
         fastmap       identify super-maximal exact matches
         pemerge       merge overlapping paired ends (EXPERIMENTAL)
         aln           gapped/ungapped alignment
         samse         generate alignment (single ended)
         sampe         generate alignment (paired ended)
         bwasw         BWA-SW for long queries

         shm           manage indices in shared memory
         fa2pac        convert FASTA to PAC format
         pac2bwt       generate BWT from PAC
         bwtupdate     update .bwt to the new format
         bwt2sa        generate SA from BWT and Occ

Note: To use BWA, you need to first index the genome with `bwa index'.
      There are three alignment algorithms in BWA: `mem', `bwasw', and
      `aln/samse/sampe'. If you are not sure which to use, try `bwa mem'
      first. Please `man ./bwa.1' for the manual.

If it does not cause regression for the supported platforms I'd be thankful if it is merged!

asl commented 6 months ago

Well, it seems it fails build non systems with unsigned char type. Let me think a bit about better solution.

martin-g commented 6 months ago

How about using 127 instead of 255 ? AFAIU any other value than the meaningful ones (1, 2, 3 and 4) should be OK ?!

On Fri, 5 Apr 2024 at 9:13, Anton Korobeynikov @.***> wrote:

Well, it seems it fails build non systems with unsigned char type. Let me think a bit about better solution.

— Reply to this email directly, view it on GitHub https://github.com/ablab/spades/pull/1269#issuecomment-2039023529, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABYUQWWQ5YBFUGK2H53BWDY3Y6KJAVCNFSM6AAAAABFXDKZOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZZGAZDGNJSHE . You are receiving this because you authored the thread.Message ID: @.***>

asl commented 6 months ago

How about using 127 instead of 255 ? AFAIU any other value than the meaningful ones (1, 2, 3 and 4) should be OK

Yes, but let me think a bit more. Maybe we'd solve the problem the other way :)

asl commented 5 months ago

I pushed a fix in https://github.com/ablab/spades/commit/d28746b666ce3627a57fd67fb83916540dc2213c

Will you please check on aarch64/linux?

martin-g commented 5 months ago

I pushed a fix in d28746b

Will you please check on aarch64/linux?

Confirmed that it works fine on Linux ARM64! Thank you! Closing this PR!

Do you have any ETA when next will be released ?

asl commented 5 months ago

Do you have any ETA when next will be released ?

Well... as soon as we will finish the documentation revamp. And finalize the release CI

asl commented 4 months ago

Just in case, SPAdes 4.0 was released: https://github.com/ablab/spades/releases/tag/v4.0.0

martin-g commented 4 months ago

Thanks! I've updated the Bioconda recipe - https://github.com/bioconda/bioconda-recipes/pull/46731

asl commented 4 months ago

Thanks! You may want to enable NCBI SRA input there. Also, it would make sense to update URL links as cab.spbu.ru is not working anymore

martin-g commented 3 months ago

You may want to enable NCBI SRA input there.

Sure! What exactly do you suggest to do ?

asl commented 3 months ago

You may want to enable NCBI SRA input there.

Sure! What exactly do you suggest to do ?

https://ablab.github.io/spades/installation.html#enabling-ncbi-sra-input-file-support

martin-g commented 3 months ago

https://github.com/bioconda/bioconda-recipes/pull/48281