ENH: add support in f2py to declare gil-disabled support #26981

ngoldbaum · 2024-07-18T18:25:04Z

This adds two new flags to the f2py CLI: --[no-]requires-gil, defaulting to --requires-gil. This allows f2py to generate modules that declare they support running without the GIL. I added tests and docs and can expand on those if anyone has suggestions.

As far as the thread safety of the f2py wrapper code, I did an audit on the lapack wrappers in f2py and some test wrappers I generated based on the f2py tests. I didn't see any thread safety issues there. There is also already special handling to make sure callbacks are thread safe. See this gist where Ralf and I were looking at this.

This also adds some support in NumPy's tests to make sure the GIL never gets re-enabled during the tests. Happy to split off the testing changes from the f2py changes if they are controversial.

It uses pytest.exit() and when it fails it looks like this:

± spin test -m full numpy/f2py/tests/test_abstract_interface.py
Invoking `build` prior to running tests:
$ /Users/goldbaum/.pyenv/versions/3.13.0b3t/bin/python vendored-meson/meson/meson.py compile -C build
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /Users/goldbaum/.pyenv/versions/3.13.0b3t/bin/ninja -C /Users/goldbaum/Documents/numpy/build
ninja: Entering directory `/Users/goldbaum/Documents/numpy/build'
[1/1] Generating numpy/generate-version with a custom command
Saving version to numpy/version.py
$ /Users/goldbaum/.pyenv/versions/3.13.0b3t/bin/python vendored-meson/meson/meson.py install --only-changed -C build --destdir ../build-install
$ export PYTHONPATH="/Users/goldbaum/Documents/numpy/build-install/usr/lib/python3.13/site-packages"
$ export PYTHONPATH="/Users/goldbaum/Documents/numpy/build-install/usr/lib/python3.13/site-packages"
$ /Users/goldbaum/.pyenv/versions/3.13.0b3t/bin/python -P -c 'import numpy'
$ cd /Users/goldbaum/Documents/numpy/build-install/usr/lib/python3.13/site-packages
$ /Users/goldbaum/.pyenv/versions/3.13.0b3t/bin/python -P -m pytest --import-mode=importlib --pyargs numpy/f2py/tests/test_abstract_interface.py
=============================================================== test session starts ===============================================================
platform darwin -- Python 3.13.0b3, pytest-8.2.2, pluggy-1.5.0
rootdir: /Users/goldbaum/Documents/numpy
configfile: pytest.ini
plugins: repeat-0.9.3, hypothesis-6.104.2
collected 2 items

numpy/f2py/tests/test_abstract_interface.py ..                                                                                              [100%]

================================================================= GIL re-enabled ==================================================================
The GIL was re-enabled at runtime during the tests.
This can happen with no test failures if the RuntimeWarning
raised by Python when this happens is filtered by a test.

Please ensure all new C modules declare support for running
without the GIL. Any new tests that intentionally imports code
that re-enables the GIL should do so in a subprocess.
Exit: GIL re-enabled during tests

In that example I forced the failure by updating to the main branch and applied the conftest.py changes from this PR.

So, no regular final pytest output about how many tests passed, failed, xpassed, etc, just a message saying Exit: GIL re-enabled during tests. Unfortunately I don't see a nicer way to fail the whole test suite after the tests are finished running using the pytest API, but maybe someone knows a better way?

rgommers · 2024-07-19T10:57:31Z

Thanks @ngoldbaum! @HaoZeke you may want to have a quick peek at the new CLI flag to see you're happy with it.

HaoZeke

Thanks for the ping @rgommers this looks good but it would be nicer to rework the added argument into the existing argparse methods used in run_compile instead, @ngoldbaum.

Basically adding to the logic in here:

numpy/numpy/f2py/f2py2e.py

Lines 553 to 587 in 472b938

    
           def make_f2py_compile_parser(): 
        
               parser = argparse.ArgumentParser(add_help=False) 
        
               parser.add_argument("--dep", action="append", dest="dependencies") 
        
               parser.add_argument("--backend", choices=['meson', 'distutils'], default='distutils') 
        
               parser.add_argument("-m", dest="module_name") 
        
               return parser 
        
           def preparse_sysargv(): 
        
               # To keep backwards bug compatibility, newer flags are handled by argparse, 
        
               # and `sys.argv` is passed to the rest of `f2py` as is. 
        
               parser = make_f2py_compile_parser() 
        
               args, remaining_argv = parser.parse_known_args() 
        
               sys.argv = [sys.argv[0]] + remaining_argv 
        
               backend_key = args.backend 
        
               if MESON_ONLY_VER and backend_key == 'distutils': 
        
                   outmess("Cannot use distutils backend with Python>=3.12," 
        
                           " using meson backend instead.\n") 
        
                   backend_key = "meson" 
        
               return { 
        
                   "dependencies": args.dependencies or [], 
        
                   "backend": backend_key, 
        
                   "modulename": args.module_name, 
        
               } 
        
           def run_compile(): 
        
               """ 
        
               Do it all in one call! 
        
               """ 
        
               import tempfile 
        
               # Collect dependency flags, preprocess sys.argv 
        
               argy = preparse_sysargv()

numpy/f2py/f2py2e.py

seberg · 2024-07-19T11:54:04Z

doc/source/f2py/usage.rst

+  is ``--requires-gil`` for backwards compatibility. Inspect the fortran
+  code you are wrapping for thread safety issues before passing
+  ``--no-requires-gil``, as ``f2py`` does not analyze fortran code for thread
+  safety issues.


There were two things that came up on the other PR. One was the idea to align with cython and use --freethreading-compatible, not sure about the merit, but as such aligning sounds like a good idea.

The other was that existing threadsafe markers are already sufficient to mark a module as compatible if all functions are marked.
So unless you actually don't like GIL release (due to overhead?), in all cases where you pass --no-requires-gil it makes sense to also mark all functions as threadsafe for the GIL enabled builds.
(If you do pass --requires-gil you will get a RuntimeWarning either way, I think; the downside being that I suppose that runtime warning will tell you the module, but less about how to adapt.)

I looked at this today and it's a little more complicated than just assuming that if the GIL is disabled, we can mark functions as threadsafe using the f2py statement. In particular, the handling for callbacks uses the PyThreadState C API, which cannot be accessed inside Py_BEGIN_ALLOW_THREADS blocks. In practice this means that python dies during the callback tests because the C API detects that it's in a Py_BEGIN_ALLOW_THREADS block.

My understanding is that in the free-threaded build, the adding the Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS would be an optimization (in the free-threaded build, it makes threads go into DETACHED state, which makes it a little easier to stop the world for a GC pass), but is not required.

I could restrict adding the macros to code that doesn't use callbacks, but I'm also worried about adding threadsafe to more wrapped fortran code, given at least as far as I can tell, it doesn't appear in any of the pyf files in the f2py tests.

The other was that existing threadsafe markers are already sufficient to mark a module as compatible if all functions are marked.

I'll take a look at this though, this seems reasonable.

So I looked at SciPy, and there is only one module (the dfitpack wrappers) where all the functions are marked thread safe. So I don't think it's worth the added complexity in the f2py code to do this, since users can just pass the CLI flag if they want to have free-threading support. I also think manually opting in isn't a bad idea either.

since users can just pass the CLI flag if they want to have free-threading support.

Right, but for the next few years at least it would be nice if you could conveniently get GIL release as well. This way you get free-threaded, I guess, but these should probably already release the GIL?

(have to come back to think about it a bit, maybe it all just doesn't matter much, but it seems like easy to run fully free, but surprsingly hard to release the GIL even though that is roughly the same for fortran code.)

I agree in principle, but in practice the f2py wrappers are not written in a way that makes it easy to release the GIL, at least for the code that handles callbacks. I'd also be a little nervous about packages suddenly shipping fortran modules that release the GIL on GIL-enabled builds when they didn't do that before, just because the package added support for free-threaded builds. I agree that most of the time it will be fine, but up until now people have had to opt in. I may also be worrying about nothing, it just feels slightly risky to me given the stability of f2py in general.

ngoldbaum · 2024-07-19T18:44:34Z

The last push renames the command-line argument from requires-gil to freethreading-compatible to align with Cython.

ngoldbaum · 2024-07-22T19:05:48Z

numpy/f2py/f2py2e.py

@@ -615,7 +630,7 @@ def run_compile():
        sysinfo_flags = [f[7:] for f in sysinfo_flags]

    _reg2 = re.compile(
-        r'--((no-|)(wrap-functions|lower)|debug-capi|quiet|skip-empty-wrappers)|-include')
+        r'--((no-|)(wrap-functions|lower|freethreading-compatible)|debug-capi|quiet|skip-empty-wrappers)|-include')


because of the argv munging that happens in this function, I also need to modify this regex, in addition to your suggestion @HaoZeke. Some of the tests only do something interesting on the free-threaded python build, which I'm guessing you don't have to test with.

Thanks for your suggestion, it didn't occur to me to just make get_includes a more generic interface.

HaoZeke

LGTM, thanks for updating the flags @ngoldbaum.

mattip · 2024-07-24T04:44:30Z

Thanks @ngoldbaum and @HaoZeke for the review.

ngoldbaum added 2 commits July 18, 2024 12:04

ENH: add support in f2py to declare gil-disabled support

c6a787e

TST: do not set PYTHON_GIL in tests

0c7564d

ngoldbaum added 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes 39 - free-threading PRs and issues related to support for free-threading CPython (a.k.a. no-GIL, PEP 703) labels Jul 18, 2024

github-actions bot added the 01 - Enhancement label Jul 18, 2024

ngoldbaum added component: numpy.f2py and removed 01 - Enhancement labels Jul 18, 2024

ngoldbaum added this to the 2.1.0 release milestone Jul 18, 2024

ngoldbaum added 2 commits July 18, 2024 13:57

MAINT: fix linter

e4f8680

TST: run new test on Python 3.12 and newer to avoid deprecations

2b1daf0

rgommers self-requested a review July 19, 2024 10:55

HaoZeke requested changes Jul 19, 2024

View reviewed changes

numpy/f2py/f2py2e.py Outdated Show resolved Hide resolved

seberg reviewed Jul 19, 2024

View reviewed changes

ngoldbaum added 2 commits July 19, 2024 12:35

MAINT: rename requires-gil to freethreading-compatible

a5b6519

MAINT: add release note

26277f5

ngoldbaum added 2 commits July 19, 2024 12:49

TST: fix errant print in test

ecf44cb

MAINT: apply rohit's refactoring

39ef0a8

ngoldbaum commented Jul 22, 2024

View reviewed changes

HaoZeke approved these changes Jul 22, 2024

View reviewed changes

ngoldbaum removed the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Jul 24, 2024

mattip merged commit cbb14a4 into numpy:main Jul 24, 2024
68 checks passed

This was referenced Jul 24, 2024

Status of NumPy compatibility Quansight-Labs/free-threaded-compatibility#2

Closed

ENH: Support free-threaded python build (tracking issue) #26157

Closed

CI: unify free-threaded wheel builds with other builds #27043

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ENH: add support in f2py to declare gil-disabled support #26981

ENH: add support in f2py to declare gil-disabled support #26981

Uh oh!

ngoldbaum commented Jul 18, 2024 •

edited

Loading

Uh oh!

rgommers commented Jul 19, 2024

Uh oh!

HaoZeke left a comment

Uh oh!

Uh oh!

seberg Jul 19, 2024

Uh oh!

ngoldbaum Jul 19, 2024 •

edited

Loading

Uh oh!

ngoldbaum Jul 19, 2024

Uh oh!

ngoldbaum Jul 19, 2024 •

edited

Loading

Uh oh!

seberg Jul 19, 2024

Uh oh!

ngoldbaum Jul 19, 2024 •

edited

Loading

Uh oh!

ngoldbaum commented Jul 19, 2024

Uh oh!

ngoldbaum Jul 22, 2024

Uh oh!

HaoZeke left a comment

Uh oh!

Uh oh!

mattip commented Jul 24, 2024

Uh oh!

Uh oh!

	def make_f2py_compile_parser():
	parser = argparse.ArgumentParser(add_help=False)
	parser.add_argument("--dep", action="append", dest="dependencies")
	parser.add_argument("--backend", choices=['meson', 'distutils'], default='distutils')
	parser.add_argument("-m", dest="module_name")
	return parser

	def preparse_sysargv():
	# To keep backwards bug compatibility, newer flags are handled by argparse,
	# and `sys.argv` is passed to the rest of `f2py` as is.
	parser = make_f2py_compile_parser()

	args, remaining_argv = parser.parse_known_args()
	sys.argv = [sys.argv[0]] + remaining_argv

	backend_key = args.backend
	if MESON_ONLY_VER and backend_key == 'distutils':
	outmess("Cannot use distutils backend with Python>=3.12,"
	" using meson backend instead.\n")
	backend_key = "meson"

	return {
	"dependencies": args.dependencies or [],
	"backend": backend_key,
	"modulename": args.module_name,
	}

	def run_compile():
	"""
	Do it all in one call!
	"""
	import tempfile

	# Collect dependency flags, preprocess sys.argv
	argy = preparse_sysargv()

Uh oh!

ENH: add support in f2py to declare gil-disabled support #26981

ENH: add support in f2py to declare gil-disabled support #26981

Uh oh!

Conversation

ngoldbaum commented Jul 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgommers commented Jul 19, 2024

Uh oh!

HaoZeke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seberg Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Jul 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Jul 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seberg Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Jul 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngoldbaum commented Jul 19, 2024

Uh oh!

ngoldbaum Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

HaoZeke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattip commented Jul 24, 2024

Uh oh!

Uh oh!

ngoldbaum commented Jul 18, 2024 •

edited

Loading

ngoldbaum Jul 19, 2024 •

edited

Loading

ngoldbaum Jul 19, 2024 •

edited

Loading

ngoldbaum Jul 19, 2024 •

edited

Loading