kcleal / pywfa

Python wrapper for wavefront alignment using WFA2-lib
MIT License
35 stars 2 forks source link

Running Overview example with conda pywfa package outputs "Illegal instruction", crashes #12

Closed matthuska closed 8 months ago

matthuska commented 9 months ago

I'm trying to use the pywfa package from bionconda and it is causing an immediate crash with the message "Illegal instruction". If I clone the repo and install pywfa code using pip it works. I'm using Ubuntu 20.04.5 LTS and the CPU is "AMD EPYC 7402 24-Core Processor".

Steps to reproduce: 1) Create a conda env with pywfa: conda create -n pywfa pywfa (currently it installs pywfa 0.5.1, build py39hf95cd2a_2) 2) Activate the conda environment 3) Copy and paste the example code from the Overview into a file, and run it

It crashes on the line with: a = WavefrontAligner(pattern)

kcleal commented 9 months ago

Hi @matthuska, That's unexpected. Does it work if you install from pypi using pip install pywfa?

matthuska commented 9 months ago

Installing from pypi also works fine, it seems that only the conda package is broken (for me at least).

kcleal commented 9 months ago

Hmm. I'm guessing it's something specific to your system. I'll leave the issue open for now, thanks for reporting

matthuska commented 9 months ago

I just tried on my laptop instead of the server and the conda package works perfectly there. Strange. If I can figure out how to get gdb running with python I can see what the illegal instruction is and report back, but it's currently a low priority.

kcleal commented 9 months ago

Thanks, tbh I probably won't have time to fix it either. If it builds from source then I'm happy enough!

matthuska commented 8 months ago

Just in case you ever have time to come back to this, I think the error is caused by the CFLAGS set in the WFA2-lib make file being set to "-march=native", which will include all optimizations available on the CPU where the code is being compiled:

https://github.com/kcleal/pywfa/blob/734c970eaee4b9a05072e8734174187548f068ba/pywfa/WFA2_lib/Makefile#L47

By default for conda-forge packages, I think they use more conservative optimization flags to that the binaries can be run on other architectures (-march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe from what I see here). I would guess that tweaking the CFLAGS for WFA2-lib would resolve this problem.

kcleal commented 8 months ago

Thanks @matthuska, that makes sense I suppose. So you think the Makefile might be overriding the more conservative flags which are set in the condo build environment?

matthuska commented 8 months ago

Yes that would be my guess. I think it would be worth changing the CC_FLAGS in the WFA2-lib Makefile to -O2 -march=nocona -mtune=haswell and seeing what happens with the next conda package release, or even just removing everything related to CC_FLAGS in the Makefile (there are two relevant lines) and hoping the defaults are picked up. I'm not so familiar with the details of the conda-forge package build process so I can't say for sure if the latter would work as we would hope.

smarco commented 8 months ago

Note that if you limit optimization flag to -O2 (instead of -O3) the compiler will not autovectorize and, therefore, the WFA2-lib performance will suffer. I understand that portability and compatibility is paramount in conda. Nonetheless, keep an eye on performance.

matthuska commented 8 months ago

That's a good point @smarco . I'll do a small benchmark to see how -O2 compares to -O3 in my environment. Maybe it will be worth adding a small note to the README if there are significant speed differences.