search

euxhenh / cellar

Interactive software tool for the assignment of cell types in single-cell studies.
https://cellar.cmu.hubmapconsortium.org/app/cellar
MIT License
31 stars 5 forks source link

cellar crashes with "Illegal instruction" on non-AVX2 cpu [fix included] #5

Closed rowanworth closed 2 years ago

rowanworth commented 2 years ago

Hi, and thanks for sharing your work!

We're running this at DUG Technology on behalf of a client and experiencing a crash when it runs on a machine that doesn't support AVX2. The failure mode looks something like this:

$ python -v main.py 
...
# code object from '/usr/local/conda/cellar/lib/python3.8/site-packages/faiss/__pycache__/swigfaiss.cpython-38.pyc'
# extension module 'faiss._swigfaiss' loaded from '/usr/local/conda/cellar/lib/python3.8/site-packages/faiss/_swigfaiss.so'
# extension module 'faiss._swigfaiss' executed from '/usr/local/conda/cellar/lib/python3.8/site-packages/faiss/_swigfaiss.so'
import 'faiss._swigfaiss' # <_frozen_importlib_external.ExtensionFileLoader object at 0x2b5e21c82b50>
import 'faiss.swigfaiss' # <_frozen_importlib_external.SourceFileLoader object at 0x2b5e21b2f970>
import 'faiss.loader' # <_frozen_importlib_external.SourceFileLoader object at 0x2b5e2151dd00>
import 'faiss' # <_frozen_importlib_external.SourceFileLoader object at 0x2b5e2150e8b0>
# /usr/local/conda/cellar/lib/python3.8/site-packages/faiss/__pycache__/swigfaiss_avx2.cpython-38.pyc matches /usr/local/conda/cellar/lib/python3.8/site-packages/faiss/swigfaiss_avx2.py
# code object from '/usr/local/conda/cellar/lib/python3.8/site-packages/faiss/__pycache__/swigfaiss_avx2.cpython-38.pyc'
Illegal instruction

Upon closer investigation of the faiss package it does include non-AVX2 python bindings, and we tracked the problem down to this line (introduced in 8226fe1e51):

controller/cellar/core/_cluster.py:5:from faiss.swigfaiss_avx2 import compute_PQ_dis_tables_dsub2

Which pulls in the AVX2 implementation regardless of what CPU type cellar is actually running on. faiss actually imports all the symbols from swigfaiss into the top-level namespace (see faiss/loader.py), so changing cluster.py to the following is enough to fix the problem:

from faiss import compute_PQ_dis_tables_dsub2

and this will still use AVX2 when available.

euxhenh commented 2 years ago

Hi, Thank you very much for sharing this! It looks like compute_PQ_dis_tables_dsub2 was accidentally auto-imported on VS Code and it is not needed. It has now been removed.

Thank you again!