MAINT: add freethreading_compatible directive to cython build #26950
Conversation
There was a problem hiding this comment.
Seems fine, of course we need to document the caveats still.
Mainly, since f2py modules require the GIL yet, I think we need to still set the env variable in our CI. Otherwise all tests running after f2py will run threaded (which may or may not include explicit threading tests).
EDIT: To be clear, by caveats, I mean object arrays not being thread safe. I think it is an important one. Not sure if there are others, but I guess one might call all others "bugs" when they come up.
| # TODO: delete when importing numpy no longer enables the GIL | ||
| # setting to zero ensures the GIL is disabled while running the | ||
| # tests under free-threaded python | ||
| export PYTHON_GIL=0 |
There was a problem hiding this comment.
One problem here is that the f2py test still need it. So half-way along the tests suite we actually would re-enable this.
So I think this should stay for now but change the comment to say that it is for f2py?
There was a problem hiding this comment.
Ah, good point, let me check if the gil is being re-enabled in the test process or just in a subprocess.
There was a problem hiding this comment.
numpy/f2py/tests/test_assumed_shape.py::TestAssumedShapeSumExample::test_all
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /Users/goldbaum/Documents/numpy/build-install/usr/lib/python3.13/site-packages/numpy/f2py/tests/test_assumed_shape.py(19)test_all()
-> breakpoint()
(Pdb) import sys
(Pdb) sys._is_gil_enabled()
True
It looks like the f2py tests do import the test modules in-process, so you're right we still need to set the environment variables. I'll update the comments around those.
If there was a way to enable and disable the GIL at runtime we could hack around this but there isn't an API for that yet (if ever).
There was a problem hiding this comment.
I added it back and also added an additional test to manually check that import numpy doesn't print the warning about the GIL being re-enabled.
It's an example, I'd prefer to keep it because it has value for |
There was a problem hiding this comment.
LGTM. I verified locally as well that it does the correct thing. Contributors who have an older 3.1.0a0 dev version of Cython will see an error that freethreading_compatible is an unknown directive (which is unavoidable); updating to the latest nightly makes that go away.
Sebastian's comment was also addressed, and the comments are clear about what's next for f2py support. So I'll go ahead and merge this.
Thanks @ngoldbaum, and thanks for the review @seberg!
A docs PR about the state of free-threaded support will be good to have indeed. Probably should just link back to the tracking issue rather than try to be comprehensive, because things are changing fast. |
This updates the cython build configuration in numpy's tests and in
numpy.randomto addfreethreading_compatible=Truecompiler directives.With this PR, numpy no longer re-enables the GIL in the free-threaded build! 🚀
numpy.randomshould be thread safe, all low-level access to the RNG state is guarded by athreading.lockin the cython code. There may be other data races I missed but IMO it's better to add these flags now to ease downstream build configuration rather than worry about theoretical thread safety issues.The
setup.pyscript used in the cython examples tests is dead code. I updated it and manually verified it still builds the extension, but maybe we should just delete it? The test itself does apytest.skipif it can't find meson.