bluenote-1577 / sylph

ultrafast taxonomic profiling and genome querying for metagenomic samples by abundance-corrected minhash.
MIT License
185 stars 6 forks source link

Conda executable crashes #2

Open fplazaonate opened 11 months ago

fplazaonate commented 11 months ago

Hi @bluenote-1577,

I installed sylph using bioconda but it crashed immediately with the following error "Illegal instruction (core dumped)". Notably, the executable available on github works.

Best, Florian

bluenote-1577 commented 11 months ago

Hi @fplaza,

Thanks for bringing this to my attention. Since the executable works, I assume you're on linux? I just tried the conda install on my Ubuntu 16 system and it appears to be fine. If you could provide your distribution and any system specifications that would be helpful for me to try and reproduce the crash.

Thanks!

fplazaonate commented 11 months ago

I am using CentOS 8. The CPU does not have avx2 extensions.

bluenote-1577 commented 11 months ago

I'm honestly not sure what's going on here... here's what I've tried

  1. Conda install on CentOS 8 - Works fine but has AVX2
  2. Building on ARM64 architecture - Works fine, no AVX2, but not from conda (bioconda doesn't support aarch64 yet)

I need to track down a machine with no AVX2 running on x86-64. If the binary works for you, then I'd be surprised if compiling doesn't work.

I'll leave this open until I figure it out, or if someone else has a similar problem

edwardbirdlab commented 10 months ago

I also seem to have the same issue on CentOS 7 without AVX2.

The executable works for me as well, but the bioconda installation crashes immediately with "Illegal instruction (core dumped)". I am running on older hardware with AVX1.

bluenote-1577 commented 10 months ago

@edwardbirdlab thanks for confirming. Does the pre-built binary work for you too?

edwardbirdlab commented 10 months ago

Yes, the pre-built binary works without any issue.

dawnmy commented 6 months ago

Hi, I am testing sylph 0.6.0 installed via conda on my Linux server, but got the same error 1724781 illegal hardware instruction (core dumped) sylph when running it. The machine has avx and avx2. The previous version 0.5.1 worked fine on the same machine.

bluenote-1577 commented 6 months ago

@dawnmy thanks for bringing this up. I can reproduce this bug as well even on my computer.

I think this is a bioconda issue, not a sylph issue: I did not change any hardware instruction behaviour between versions. I will update to version v0.6.1 and hope that bioconda fixes this. If not, it looks like I will have to ask the conda folks for help.

bluenote-1577 commented 6 months ago

@dawnmy I updated sylph to v0.6.1 on conda. It works okay on my computer now. Let me know on your end

dawnmy commented 6 months ago

Thank you for the quick update. It works!

bluenote-1577 commented 5 months ago

I figured out what the issue is: -target=native rust flag is being passed to the bioconda build. So the binary will use native instructions on the bioconda machine that is building sylph. This is why AVX2 is needed for the current conda build.

In the future, I will probably have to disable -target=native for conda. This will maybe make sylph slower, unfortunately...

I would prefer AVX2 to be required for conda, but not AVX512, which -target=native may force.

fplazaonate commented 5 months ago

Hi @bluenote-1577, You can build different executables and choose on the fly which one is the most appropriate. Look at what's implemented for raptor: https://github.com/bioconda/bioconda-recipes/tree/master/recipes/raptor

fplazaonate commented 2 months ago

Hi @bluenote-1577,

Here is a first implementation that works (tested on computers with or without avx-512 instruction set) https://github.com/bioconda/bioconda-recipes/compare/master...fplazaonate:bioconda-recipes:sylph_recipe

Tell me what you think about it.

bluenote-1577 commented 2 months ago

Hi @fplazaonate

I looked through it and it seems great. I haven't tested it but I trust your testing.

My major thing: It doesn't support aarch64 builds? I believe sylph should build fine on aarch64 (skani aarch64 conda builds fine). While sylph is not enabled for aarch64 in conda yet, it would be nice in the future if it did.

Feel free to make a pull request. Thanks so much.

Jim

fplazaonate commented 2 months ago

Thanks for your feedback. Here is a PR that add support for aarch64 CPU: https://github.com/bioconda/bioconda-recipes/pull/50462