Test using pytest-run-parallel and related fixups in the tests - #2194
Test using pytest-run-parallel and related fixups in the tests#2194seberg wants to merge 7 commits into
Conversation
|
/ok to test ed40f60 |
|
|
Fun, the refactor made the cufile xfail-strict tests pass on CI, but I didn't set up the parallel run correctly... one more try: /ok to test eb6a2ff |
|
/ok to test eb6a2ff |
|
/ok to test 7b59bff |
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
7d33149 to
4f2cc7a
Compare
|
Still quite a few changes in this now (and let's see if CI passes) including gh-2321. But maybe it's still fine to review. The most annoying one is probably the |
d9d8553 to
6847e09
Compare
| if python -c 'import sys; assert not sys._is_gil_enabled()' 2> /dev/null; then | ||
| FREE_THREADING="-ft" | ||
| PYTEST_PARALLEL_ARGS=(--parallel-threads=4) | ||
| pip install pytest-run-parallel |
There was a problem hiding this comment.
Can you add this to the test groups in the normal places under cuda_{bindings,core,pathfinder}/pyproject.toml?
There was a problem hiding this comment.
Makes sense, I added it as a test-ft but it currently only contains pytest-run-parallel. Happy to do it differently.
(Semi-related, removed the special logic for cupy not support free-threaded as it does now.)
Signed-off-by: Sebastian Berg <sebastianb@nvidia.com>
This also seems to fix the issues around memory resource finishing/cleanup. I.e. the main part of the issue was apparently issueing device syncs from all threads. (Clean-up in a follow-up.)
seberg
left a comment
There was a problem hiding this comment.
(Few comments for review 🤞 that it will run through successfully in CI with all the drift going on.)
| # never enqueued because their owning object had not been collected yet. | ||
| gc.collect() | ||
| driver.cuCtxSynchronize() | ||
| _ = _device_unset_current() |
There was a problem hiding this comment.
This was causing issues for some graph tests (because a device sync disrupts graph capture).
But, actually, it also seems to fix the mr cleanup issue... I.e. the dead-lock I was seeing happens only if there is a device synchronization on each thread.
That should allow a bunch of cleanups, but I would prefer to follow-up for it.
| @pytest.mark.agent_authored(model="gpt-5.6") | ||
| @pytest.mark.parametrize("fixture_name", CLOSEABLE_TYPES) | ||
| def test_closeable_object_state_and_safe_inspection(fixture_name, request): | ||
| @pytest.mark.thread_unsafe(reason="closes a fixture object shared between threads") |
There was a problem hiding this comment.
Just marked as unsafe here, but also changed to follow the same indirect fixture pattern as the rest of the file.
| info = cuda_stream_attr() | ||
| else: | ||
| info = cuda_stream_attr | ||
| warnings.simplefilter("once", DeprecationWarning) |
There was a problem hiding this comment.
This triggered a failure. Could also just mark the test as unsafe, but mutating global state here seems wrong (e.g. what if the user wants to raise DeprecationWarnings?).
| if python -c 'import sys; assert not sys._is_gil_enabled()' 2> /dev/null; then | ||
| FREE_THREADING="-ft" | ||
| PYTEST_PARALLEL_ARGS=(--parallel-threads=4) | ||
| pip install pytest-run-parallel |
There was a problem hiding this comment.
Makes sense, I added it as a test-ft but it currently only contains pytest-run-parallel. Happy to do it differently.
(Semi-related, removed the special logic for cupy not support free-threaded as it does now.)
Description
Enable testing with
pytest-run-parallelon CI for 3.14t and 3.15t as well some residual related fixes.Checklist