pola-rs / polars

Dataframes powered by a multithreaded, vectorized query engine, written in Rust
https://docs.pola.rs
Other
30.57k stars 1.98k forks source link

POLARS_SKIP_CPU_CHECK doesn't prevent illegal hardware instruction even with bare import anymore #19936

Closed mishamsk closed 5 days ago

mishamsk commented 5 days ago

Checks

Reproducible example

admin@alchemist-vm-direct-python311 test-pytest % uv venv
Using CPython 3.11.10 interpreter at: /Users/admin/.asdf/installs/python/3.11.10/bin/python
Creating virtual environment at: .venv
Activate with: source .venv/bin/activate
admin@alchemist-vm-direct-python311 test-pytest % uv pip install polars
Resolved 1 package in 6ms
Installed 1 package in 8ms
 + polars==1.14.0
admin@alchemist-vm-direct-python311 test-pytest % source .venv/bin/activate     
(test-pytest) admin@alchemist-vm-direct-python311 test-pytest % export POLARS_SKIP_CPU_CHECK=1
(test-pytest) admin@alchemist-vm-direct-python311 test-pytest % export POLARS_VERBOSE=1
(test-pytest) admin@alchemist-vm-direct-python311 test-pytest % python -c "import polars"     
zsh: illegal hardware instruction  python -c "import polars"
(test-pytest) admin@alchemist-vm-direct-python311 test-pytest % 

Log output

zsh: illegal hardware instruction  python -c "import polars"

Issue description

We've recently upgraded from 0.20.21 to 1.12.0 and hit a regression. We are using a pretty old machine for our macos runner to build our app, which doesn't have AVX instruction, hence wouldn't pass the cpu check.

However, the app is built to be run on modern CPU's, so it was never an issue. Runner would never fail, since non of the test really hit anything that would trigger a runtime issue with instructions. So we were all good.

Now, polars seems to do something very early during import that causes an error. And setting POLARS_SKIP_CPU_CHECK doesn't really help. It would only silence the warning in _cpu_check.py, but some code does something that would trigger a failure just from import...

Unfortunately, we can't use polars-lts-cpu, since whatever is on the runner gets bundled into the app, and I wouldn't want the lts version ending up on all Macs.

I do understand that upgrading the runner is one solution, but was wondering if this should now be considered an expected behavior that even bare import immediately triggers illegal instruction error?

Expected behavior

Polars doesn't try to execute some cpu-feature-dependant code just to be imported.

Installed versions

Can't show via pl, command, but here is the closest I can get manually: ``` Package Version ------- ------- polars 1.14.0 macosx-12.6-x86_64 ```
ritchie46 commented 5 days ago

You are probably running under rosetta which emulates a very old CPU instruction set and doesn't support the instruction set Polars compiles to. Either use polars-lts-cpu or don't run under rosetta.

See: https://github.com/pola-rs/polars/issues/12454

mishamsk commented 5 days ago

@ritchie46 hi! I did the search for earlier issues before opening this one, I saw the rosetta thing. But thanks for sharing the link.

Our case is not Rosetta. The runner is MacOs inside Parallels running on Mac Pro from 2003. The host has old Xeon CPU. I think this CPU actually lacks some instructions sets that you check for/use. So generally non lts-cpu polars can fail, I would expect that if some operation is executed that uses modern instruction set. However, it seems that our usage pattern didn’t trigger any issues before and polars would work ok despite the warning.

Now, just importing polars immediately runs something that causes the failure. So I am just asking if this is intended to be so or something creeped in during import accidentally? If the current behavior is as intended feel free to just close the issue.

PS. I think we’ll just have to temporarily bundle lts-cpu version for all x86 macs and then upgrade our runner host, so we’ll workaround this.

ritchie46 commented 5 days ago

Now, just importing polars immediately runs something that causes the failure. So I am just asking if this is intended to be so or something creeped in during import accidentally? If the current behavior is as intended feel free to just close the issue.

Intended is a big word. The import fails because Polars runs some start up instructions which are not supported by that very old CPU and does you get the illegal instruction abort. The only thing we can do sometimes is detect that before importing the binary and give a nicer error. Either way, you need polars-lts-cpu to run on that machine.

Hopefully we can gracefully handle that in the future by installing both and picking lts if the default is not supported.

mishamsk commented 5 days ago

thanks for the context, really appreciate you taking the time to dive into this! And thanks a lot for all the great work. Big fan/advocate of polars!

Will close the issue now.