search

isovic / racon

Ultrafast consensus module for raw de novo genome assembly of long uncorrected reads. http://genome.cshlp.org/content/early/2017/01/18/gr.214270.116 Note: This was the original repository which will no longer be officially maintained. Please use the new official repository here:
https://github.com/lbcb-sci/racon
MIT License
269 stars 49 forks source link

Make compiler flags opional instead of being auto detected #142

Open jueneman opened 5 years ago

jueneman commented 5 years ago

This is more related to the dependent projects clara-genomics and spoa, but therefore also true for racon therefore If you compile a version on Intel CPUs, the binary wont work on AMD CPUs, as it now uses part of the AVX-512 instruction set (e.g. VCVTTSS2SI), which isn't available for AMD architecture. If you have a mixed infrastructure or have no control about what machine you are finally going to use in terms of supported CPU instruction sets (HPC, cloud, ...), then this might be problematic.

I would suggest to provide an additional installation (cmake) flag which allows to disable any architecture bound compilation and not just testing for the present arch (if defined (AVX2) and such) and automatically choosing how to build the binary.

Of course you could always re-install from the scratch, but thats also not always possible or easy to maintain.

THX!

rvaser commented 5 years ago

Hi Sebastian, the bioconda executable is built with SSE4.1 (by adding -DCMAKE_CXX_FLAGS="-mno-avx2") and without CUDA support. Should we add something for AVX512 as well? Spoa has a CMake flag called spoa_optimize_for_native which can be used to remove -march=native. Is that sufficient or do you have something else in mind?

Sorry for the wait! Best regards, Robert

jueneman commented 5 years ago

Hi Robert,

we were not using conda. i have nothing special in my mind, but including AVX512 as well sounds quite reasonable for me, as i don't see why excluding AVX2 should also include AVX512. Anyhow, i have not tested this (if both options would be exclusive or not), so I dont know if thats even necessary.

Thx for the hint on -DCMAKE_CXX_FLAGS="-mno-avx2", that might actually already work.

rvaser commented 5 years ago

I'll see what I can do to make compilation easier!

ttubb commented 4 years ago

Hi Sebastian, the bioconda executable is built with SSE4.1 (by adding -DCMAKE_CXX_FLAGS="-mno-avx2") and without CUDA support. Should we add something for AVX512 as well? Spoa has a CMake flag called spoa_optimize_for_native which can be used to remove -march=native. Is that sufficient or do you have something else in mind?

Sorry for the wait! Best regards, Robert

Would you consider doing it the other way around - by default compiling in a way that is least likely to cause compatibility issues? Otherwise this seems like quite a nasty pitfall for some use cases. I use racon in containers, with the images being built on dockerhub. Meaning when i update the Dockerfile in my repository, i might get a different outcome regarding AVX512 just because dockerhub uses a different machine. I run workflows in the cloud, so CPU-types vary. It took months of pipelines occasionally failing until i figured out the issue.

rvaser commented 4 years ago

Are you referring to Bioconda executable or compilation from source?

ttubb commented 4 years ago

Are you referring to Bioconda executable or compilation from source?

Compilation from source (same as jueneman)

rvaser commented 4 years ago

I think it is doable, will get back to you. I am swamped until end of October :)

jueneman commented 4 years ago

Hi Sebastian, the bioconda executable is built with SSE4.1 (by adding -DCMAKE_CXX_FLAGS="-mno-avx2") and without CUDA support. Should we add something for AVX512 as well? Spoa has a CMake flag called spoa_optimize_for_native which can be used to remove -march=native. Is that sufficient or do you have something else in mind? Sorry for the wait! Best regards, Robert

Would you consider doing it the other way around - by default compiling in a way that is least likely to cause compatibility issues? Otherwise this seems like quite a nasty pitfall for some use cases. I use racon in containers, with the images being built on dockerhub. Meaning when i update the Dockerfile in my repository, i might get a different outcome regarding AVX512 just because dockerhub uses a different machine. I run workflows in the cloud, so CPU-types vary. It took months of pipelines occasionally failing until i figured out the issue.

This is more or less exactly the problem we run into. Applying this in cloud use cases can cause some issues - as pointed out, machine hardware can change and is unreliable.