Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make AVX2-detection platform-independent #1600

Closed
wants to merge 6 commits into from
Closed
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions faiss/python/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,52 @@
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

from distutils.version import LooseVersion
import platform
import subprocess
import logging


def instruction_set():
"""
Returns a dictionary for supported instruction sets, see
https://github.com/numpy/numpy/blob/master/numpy/core/src/common/npy_cpu_features.h
for the list of features that this dictionary contains per architecture.

Example:
>>> instruction_set() # for x86
{"SSE2": True, "AVX2": False, ...}
>>> instruction_set() # for PPC
{"VSX": True, "VSX2": False, ...}
>>> instruction_set() # for ARM
{"NEON": True, "ASIMD": False, ...}
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is constrained on the numpy side but what about returning a set with the flags set to True?

For example, below the has_AVX2 check will fail on ARM with KeyError.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about the key error. I think the cleanest fix would just be to use .get("AVX2", False) on the output, which is fine even if some keys are not there.

Alternatively, we could "pad" the dictionary with the keys from the other architectures with value False. I don't like this idea though, because we'd be duplicating the list of instruction sets compared to numpy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also return a set in every case (in the case of numpy, selecting the keys for which the value is True).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that's a good idea as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for using the set. Will get it through the FB internal checks.

import numpy
if LooseVersion(numpy.__version__) >= "1.19":
# use private API as next-best thing until numpy/numpy#18058 is solved
from numpy.core._multiarray_umath import __cpu_features__
return __cpu_features__

# platform-dependent legacy fallback before numpy 1.19, no windows
if platform.system() == "Darwin":
if subprocess.check_output(["/usr/sbin/sysctl", "hw.optional.avx2_0"])[-1] == '1':
return "AVX2"
return {"AVX2": True}
else:
return "default"
return {"AVX2": False}
elif platform.system() == "Linux":
import numpy.distutils.cpuinfo
if "avx2" in numpy.distutils.cpuinfo.cpu.info[0].get('flags', ""):
return "AVX2"
return {"AVX2": True}
else:
return "default"
return {"AVX2": False}
return {"AVX2": False}


logger = logging.getLogger(__name__)

try:
instr_set = instruction_set()
if instr_set == "AVX2":
has_AVX2 = instruction_set()["AVX2"] # dict-values of instruction_set() are True or False
if has_AVX2:
logger.info("Loading faiss with AVX2 support.")
from .swigfaiss_avx2 import *
else:
Expand Down