Enable free threaded python support - #159
Conversation
| # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
|
|
||
| # cython: language_level=3 | ||
| # cython: freethreading_compatible=True |
There was a problem hiding this comment.
We need to make changes in various places in this file, there are several areas where we are likely creating race conditions. Some that were apparent in review and with some thinking: multinomial, leapfrog, skipahead, multinormal_cholesky, and the two randint_untyped branches for scalars
For example, we need with self.lock: wrapping irk_multinomial_vec in multinomial
There was a problem hiding this comment.
We likely need more changes throughout the file
There was a problem hiding this comment.
@ndgrigorian great catch, I missed these
You are right, they race without self.lock
Fixed, thanks!
…o enable-free-threaded-python
|
|
||
| def test_concurrent_sampling_per_instance(): | ||
| # Each thread owns a private MKLRandomState seeded identically, so the | ||
| # per-instance lock + `nogil` sampling must reproduce the single-threaded |
There was a problem hiding this comment.
Each thread builds its own MKLRandomState — no shared state, so it passes with or without the per-instance lock. Fine as an instance-independence smoke test, but the docstring's "exercises the per-instance lock" claim is misleading.
| k, rounds, seed = 32, 20, 777 | ||
| rs = mkl_random.MKLRandomState(seed) | ||
| rs.seed(seed) | ||
| ref = Counter(repr(call(rs)) for _ in range(k)) |
There was a problem hiding this comment.
For the multinomial and mvn_cholesky params repr truncates precision, weakening race detection for those two
| assert r.shape == (size,) | ||
| assert np.all(np.isfinite(r)) |
There was a problem hiding this comment.
Assertions are too weak. A data race that interleaves stream updates still produces finite floats in [0,1), so the corruption passes.
| np.testing.assert_array_equal(r, expected) | ||
|
|
||
|
|
||
| def test_concurrent_shared_singleton(): |
There was a problem hiding this comment.
Not FT-gated. On a GIL build it runs as a pure no-op smoke test (the GIL serializes it anyway), giving false confidence.
| np.testing.assert_array_equal(r, expected) | ||
|
|
||
|
|
||
| def test_concurrent_shared_singleton(): |
There was a problem hiding this comment.
No barrier — unlike _draw_concurrently, the threads aren't released together, so contention on the stream-touching region is minimal and the race window is barely exercised.
There was a problem hiding this comment.
operator.index(seed), np.asarray, astype can run user __index__/__array__ that re-enters the generator → self-deadlock
3 participants
This PR proposes enabling free-threaded Python support in
mkl_randomand makes the RNG safe for concurrent usemklrandwith# cython: freethreading_compatible=Trueso importing the package no longer re-enables the GIL.Cython>=3.1.0(pyproject.toml, conda recipes, README) and addFree Threading :: 2 - Beta classifierpython-gilpin from the conda recipes.free-threading testand3.14t / *_cp314tvariants to the GH workflow matricessize=Nonedraws,multinomial,multinormal_cholesky,shuffle,seed,randinthelpers), fix re-entrancy deadlocks and aget_stateheap overflow on concurrent BRNG changeAdditionally, fixed memory leaks in
set_state(4a0fb51) andlogseries(548d625)