Run cudf_polars unit tests with RapidsMPF - #21807
Conversation
github-actions
Bot
added
Python
mroeschke
changed the title
| # Run experimental tests with the "single" cluster mode and the "rapidsmpf" runtime | ||
| rapids-logger "Running experimental tests with the 'rapidsmpf' runtime and a 'single' cluster" | ||
| timeout 10m python -m pytest --cache-clear "$@" "tests/experimental" \ | ||
| CUDF_POLARS__PARQUET_OPTIONS__USE_RAPIDSMPF_NATIVE=1 CUDF_POLARS__EXECUTOR__SHUFFLE_METHOD=rapidsmpf timeout 10m python -m pytest --cache-clear "$@" tests \ |
There was a problem hiding this comment.
Hmm kind of annoying that all the cudf and pandas tests will run because of this change. Perhaps we can figure out how to avoid?
There was a problem hiding this comment.
Yeah I think we either need to:
- Specifically include cudf_polars, cudf, cudf.pandas etc. specific CI files in the changed files logic
- Convince folks to split the
cidirectory to be project specific e.g.python/cudf_polars/ci/,python/cudf/cietc. (probably a larger ask)
bdice
left a comment
bdice
left a comment
There was a problem hiding this comment.
This PR instead runs the entire cudf_polars test suite in the non-blocking cudf_polars rapidsmpf job as opposed to the blocking
test_python_otherand cudf_polars wheel test jobs.
I don't see how this change is being made. Don't you need to edit https://github.com/rapidsai/cudf/blob/main/ci/test_python_other.sh?
| using_rapidsmpf: bool, | ||
| ) -> None: | ||
| if using_rapidsmpf: | ||
| pytest.skip("Can segault on 12.9 builds with rapidsmpf runtime.") |
There was a problem hiding this comment.
| pytest.skip("Can segault on 12.9 builds with rapidsmpf runtime.") | |
| pytest.skip("Can segfault on 12.9 builds with rapidsmpf runtime.") |
Do we have an issue tracking this?
There was a problem hiding this comment.
Yes, updated the pytest.skips to fix the type and include the github link
| def test_boolean_horizontal(expr, has_nulls, wide): | ||
| def test_boolean_horizontal(expr, has_nulls, wide, using_rapidsmpf): | ||
| if using_rapidsmpf: | ||
| pytest.skip("Can segault on 12.9 builds with rapidsmpf runtime.") |
There was a problem hiding this comment.
This typo occurs repeatedly.
| pytest.skip("Can segault on 12.9 builds with rapidsmpf runtime.") | |
| pytest.skip("Can segfault on 12.9 builds with rapidsmpf runtime.") |
bdice
left a comment
bdice
left a comment
There was a problem hiding this comment.
This makes way more sense now. Thanks for updating the description!
| monkeypatch: pytest.MonkeyPatch, | ||
| rapidsmpf_distributed_available: bool, | ||
| ) -> None: | ||
| # TODO: Can remove once removed from ci/run_cudf_polars_pytests.sh |
There was a problem hiding this comment.
FWIW, I think keeping this is fine. If possible, our tests should be robust to what's in the environment when the test process starts.
There was a problem hiding this comment.
I'll just go ahead and revert this since I'll be removing the shuffler environment variable that affected the tests in this file.
| # Run experimental tests with the "single" cluster mode and the "rapidsmpf" runtime | ||
| rapids-logger "Running experimental tests with the 'rapidsmpf' runtime and a 'single' cluster" | ||
| timeout 10m python -m pytest --cache-clear "$@" "tests/experimental" \ | ||
| CUDF_POLARS__PARQUET_OPTIONS__USE_RAPIDSMPF_NATIVE=1 CUDF_POLARS__EXECUTOR__SHUFFLE_METHOD=rapidsmpf timeout 10m python -m pytest --cache-clear "$@" tests \ |
There was a problem hiding this comment.
Can you say why setting CUDF_POLARS__EXECUTOR__SHUFFLE_METHOD was necessary? The default method should depend on the runtime
In [8]: import polars as pl
...: from cudf_polars.utils.config import ConfigOptions
...:
...:
...: engine = pl.GPUEngine(executor="streaming", executor_options={"runtime": "rapidsmpf"})
...: cfg = ConfigOptions.from_polars_engine(engine)
...: cfg.executor.shuffle_method
...:
Out[8]: <ShuffleMethod._RAPIDSMPF_SINGLE: 'rapidsmpf-single'>There was a problem hiding this comment.
Ah fair point, I suppose I was trying to guarantee we would always use the rapidsmpf shuffler here (since there's logic for the distributed cluster falling back to the task shuffler), but I'll remove as it's not really necessary.
|
/merge |
5 participants
Description
Broken off from #21485
This PR takes a different approach from the linked PR by modifying an existing invocation in
ci/run_cudf_polars_with_rapidsmpf_pytests.shto run all cudf_polars unit tests with rapidsmpf in an existing non-blocking CI job (wheel-tests-cudf-polars-with-rapidsmpf) instead of the blockingwheel-tests-cudf-polarsandconda-python-other-testsjobs. We will want to eventually pursue the latter approach once rapidsmpf development is more stable relative to cudf_polars usage.This PR also aggressively xfails and skips tests known to fail in #21467
Checklist