-
Notifications
You must be signed in to change notification settings - Fork 1k
Add a multithreaded tokenizer test and as well as 3.14 and 3.14t CI #1864
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
base: main
Are you sure you want to change the base?
Conversation
|
Perhaps add Py3.14t? |
|
@cclauss nope, not until hf-xet updates its PyO3 dependency and/or releases cp314t wheels: |
d7f7fb7 to
d7fc265
Compare
|
Went ahead and rebased though. @Narsil any chance you can take a look over here? |
c9f12e5 to
8ea6243
Compare
|
I updated this to target 3.14t instead of 3.13t and added 3.14 testing while I was at it. I also updated a few spots where CI configuration has gone stale. After all that, the tests pass on my fork, see ngoldbaum#3. The way I set this up, the GIL is getting re-enabled in the 3.14t tests. I'll mark the extensions as supporting free-threading in a follow-up. @ArthurZucker any chance you can trigger CI over here so the tests run? |
8ea6243 to
8446484
Compare
|
Rebased on #1923. |
|
I went ahead and opened #1925 to make this PR do fewer things at once. |
Towards fixing #1784.
This is to aid supporting the free-threaded build. Adding explicitly multithreaded tests like this exercises the behavior differences of the free-threaded build compared with the GIL-enabled build. See #1809 where I initially tried this with a different testing approach.
The content of the test is adapted from the asyn
test_concurrencytest.I tried to make this use multiprocessing as well but ran into deadlocks so decided to stick to using 4 python threads in a thread pool.
c.f. huggingface/safetensors#637 where I added a multithreaded test to safetensors.