-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
"import rapidfuzz" causes "illegal hardware instruction" #383
Comments
strange nothing really changed between these two versions that should affect the C++ version. Can you provide some context:
|
Machine information: macOS 10.13.6 |
I think I have an idea what this could be. I did build the wheels using the I will make a test build using macOS 13 x86 and link you the wheel here for a test install once it's finished for you to test. |
Can you test this wheel? |
I installed the wheel with After that, running the test script still causes
|
Thanks. Can you test the following package as well? This is version 3.8.1 built with the current toolchains since I suspect this is a build issue and not an issue with the library itself. |
This also causes the error message
|
@henryiii can you have a quick look at this? You are usually more familiar with ecosystem changes in Python build tools. To give a short recap of what is happening here:
So this sounds very much like something broke due to implicit updates of build tooling. Looking at the build log I noticed a couple of differences:
I don't see any other differences between the two builds, but maybe I am missing something. Do you have any idea what could go wrong here? |
Can you give the following build a try: This is a build without the avx2 versions |
As a first step, I'd recommend using The only similar issue I've seen so far was that either uv or ruff builds with an old macOS because there was an issue using the newer Xcode when targeting older macOS. I haven't checked yet to see exactly what the issue is, they are building on macos-11 for now. (maturin-action and Rust, not cibuildwheel) I technically do have access to a macOS <11 machine (10.12 IIRC - it's an iMac with an NVIDIA graphics card, it's that old!), but it's in my old office and I won't be going there probably at least for a week. (Currently at PyCon, so def. don't have access to it now!) |
No crash from the test script 👍 |
On a branch macOS-12 worked. But sounds like this is the correct fix. Does avx make it noticeably faster? You could make a higher minimum version wheel too with avx if it was important. |
I don't see anything directly obvious on what I might have done wrong in 177663a that would lead to no build targets being selected
Which fix do you mean? Disabling avx2 using macOs-12 or something else?
When performing many x many comparisons it actually yields close to a 2x performance improvement for some metrics, since it allows me to compare twice as many strings in parallel. So I would prefer to keep it. Currently I include the binary twice and just load the correct one based on CPU feature flags.
Do you mean publishing both an 10.9+ version using only sse2 and a version targeting a later version which guarantees AVX2? If so which one does actually guarantee this? I mean the cpu in question here (i7 870) already supports AVX2. |
I don't either, but #384 passes.
I was thinking avx2, but it was previously working, so to me this sounds like a bug. What's odd though the 13+ (probably Xcode is the important part, the previous macOS always updates Xcode once to next macOS's Xcode) does seem to work except on older macOS. It would be interesting to list the binary symbols, I think. I think since macos-12 does work (in #384), the simplest thing might be to use that (assuming it fixes the problem) until GHA drops the image. I think the macos-11 image is still usable but deprecated, so it's probably ~1 year unless there are hardware reasons to drop it sooner.
Yes, but I don't know what version this would be, somewhere between 10.13 and 13; since it only happens on the actual macOS versions, it would be really hard to pin down. I rather expect this is a bug and it might get ironed out in later releases, if we could pin it down a bit more possibly could even report it. |
You forgot a quote. |
Oh true |
@workflowsguy I published a version with avx2 disabled yesterday to take a bit of the urgency out of this. |
Yes I think so too. It already fails in Cythons init functions for the module because it comes across an unsupported instruction. Since I only enable AVX2 which is supported on the CPU in question this really shouldn't occur. So maybe an empty Cython module would be a good first test. |
Thanks! |
I can't really test this myself since I don't have access to a machine that leads to the crash to try and create a minimal reproducer. However this appears to already crash pretty early on while initializing the module. So we might be able to reduce the sample by quite a bit. |
While I had used rapidfuzz previously without issues in my Python scripts, now every script that uses it crashes with an error such as
illegal hardware instruction
(Console) orTerminated due to signal: ILLEGAL INSTRUCTION (4)
(Development environment)The simplest script to demonstrate this just consists of
import rapidfuzz
Versions:
rapidfuzz 3.9.0 (installed with
pip install rapidfuzz
)Python 3.11.9
After I reverted to rapidfuzz 3.8.1, the issue did not occur anymore
Thanks!
The text was updated successfully, but these errors were encountered: