Skip to content

Reduce peak footprint of cudf-polars test memory usage#22493

Merged
rapids-bot[bot] merged 4 commits into
rapidsai:mainfrom
wence-:wence/fix/cudf-polars-test-usage
May 14, 2026
Merged

Reduce peak footprint of cudf-polars test memory usage#22493
rapids-bot[bot] merged 4 commits into
rapidsai:mainfrom
wence-:wence/fix/cudf-polars-test-usage

Conversation

@wence-
Copy link
Copy Markdown
Contributor

@wence- wence- commented May 13, 2026

Description

Previously all engines would be live simultaneously. When additionally
running tests in parallel with xdist, this results in significant
oversubscription of test resources.

To fix this, reorder test collection so that tests are order by engine
type, and only create the session-scoped engines one at a time.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@wence- wence- requested a review from a team as a code owner May 13, 2026 16:08
@wence- wence- added the improvement Improvement / enhancement to an existing function label May 13, 2026
@wence- wence- requested a review from mroeschke May 13, 2026 16:08
@wence- wence- added the non-breaking Non-breaking change label May 13, 2026
@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels May 13, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6a00d4a9-e749-4a55-a3b3-432a0b0b2640

📥 Commits

Reviewing files that changed from the base of the PR and between 483ffdc and c8619e3.

📒 Files selected for processing (11)
  • python/cudf_polars/cudf_polars/testing/asserts.py
  • python/cudf_polars/cudf_polars/testing/engine_utils.py
  • python/cudf_polars/tests/conftest.py
  • python/cudf_polars/tests/experimental/test_default_singleton_engine.py
  • python/cudf_polars/tests/experimental/test_metadata.py
  • python/cudf_polars/tests/experimental/test_sink.py
  • python/cudf_polars/tests/test_config.py
  • python/cudf_polars/tests/test_executors.py
  • python/cudf_polars/tests/test_sink.py
  • python/cudf_polars/tests/test_tracing.py
  • python/cudf_polars/tests/testing/test_engine_utils.py
✅ Files skipped from review due to trivial changes (1)
  • python/cudf_polars/tests/experimental/test_default_singleton_engine.py
🚧 Files skipped from review as they are similar to previous changes (7)
  • python/cudf_polars/tests/testing/test_engine_utils.py
  • python/cudf_polars/tests/test_tracing.py
  • python/cudf_polars/tests/test_sink.py
  • python/cudf_polars/tests/experimental/test_metadata.py
  • python/cudf_polars/tests/experimental/test_sink.py
  • python/cudf_polars/cudf_polars/testing/asserts.py
  • python/cudf_polars/tests/test_executors.py

📝 Walkthrough

Summary by CodeRabbit

  • Tests
    • Assertion helpers now require an explicit engine and no longer rely on an implicit default.
    • Streaming-engine fixtures and parametrization were refactored for clearer per-test configuration and isolation.
    • New helpers to merge and apply streaming options centralize engine configuration.
    • Several tests updated to pass explicit configured engines (including local in-memory engines).
    • Pytest wiring simplified to remove global executor defaults and streamline test collection.

Walkthrough

The PR requires explicit GPUEngine parameters for GPU/sink test assertions, adds streaming-options merge and engine-configuration helpers, centralizes unconfigured engine construction in pytest fixtures, and updates tests to pass explicit engines to assertion helpers.

Changes

Engine Fixture and Assertion Refactoring

Layer / File(s) Summary
Assertion API contract
python/cudf_polars/cudf_polars/testing/asserts.py
assert_gpu_result_equal and assert_sink_result_equal now require explicit keyword-only engine: GPUEngine. Removed DEFAULT_EXECUTOR, get_default_engine, executor-based defaulting, and executor parameters; docstrings updated.
Engine utility helpers
python/cudf_polars/cudf_polars/testing/engine_utils.py
create_streaming_options no longer accepts overrides and returns baseline options. Added merge_streaming_options to combine options and configure_streaming_engine to apply options via _reset(...). Removed build_streaming_engine and related Mapping typing.
Pytest engine fixture wiring
python/cudf_polars/tests/conftest.py
Added _engine_param and session-scoped _unconfigured_engine to construct base engines and baseline options; rebuilt spmd_engine, factories, streaming_engine, and engine fixtures to apply merge_streaming_options and configure_streaming_engine. Removed streaming_engines mapping and --executor option; added pytest_generate_tests and adjusted collection hooks.
Test updates across suite
python/cudf_polars/tests/...
Updated test signatures to accept new fixtures (streaming_engine, engine) or construct local pl.GPUEngine instances; updated assertion calls to pass engine=... instead of relying on executor strings or module defaults. Files affected include test_config.py, test_executors.py, test_sink.py, test_tracing.py, testing/test_engine_utils.py, experimental/test_metadata.py, experimental/test_sink.py, and experimental/test_default_singleton_engine.py.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • rapidsai/cudf#22381: Direct connection through shared refactoring of testing fixture wiring in conftest.py, engine_utils.py, and asserts.py.
  • rapidsai/cudf#22410: Related through changes to asserts.py and default engine handling logic.

Suggested reviewers

  • mroeschke
  • TomAugspurger
  • bdice
  • madsbk
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the primary objective of reducing peak memory footprint in cudf-polars tests by reordering collection to instantiate engines sequentially rather than simultaneously.
Description check ✅ Passed The description clearly explains the problem (all engines live simultaneously causing resource oversubscription with xdist) and the solution (reorder test collection by engine type to create session-scoped engines sequentially).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Member

@madsbk madsbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment thread python/cudf_polars/cudf_polars/testing/engine_utils.py Outdated
Comment thread python/cudf_polars/tests/conftest.py
Comment thread python/cudf_polars/tests/conftest.py Outdated
wence- added 4 commits May 14, 2026 09:52
Previously all engines would be live simultaneously. When additionally
running tests in parallel with xdist, this results in significant
oversubscription of test resources.

To fix this, reorder test collection so that tests are order by engine
type, and only create the session-scoped engines one at a time.
This fixture is only used in one place.
@wence- wence- force-pushed the wence/fix/cudf-polars-test-usage branch from 483ffdc to c8619e3 Compare May 14, 2026 09:46
@GPUtester GPUtester moved this to In Progress in cuDF Python May 14, 2026
@wence-
Copy link
Copy Markdown
Contributor Author

wence- commented May 14, 2026

/merge

@rapids-bot rapids-bot Bot merged commit 9a85bda into rapidsai:main May 14, 2026
87 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python May 14, 2026
@wence- wence- deleted the wence/fix/cudf-polars-test-usage branch May 14, 2026 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cudf-polars Issues specific to cudf-polars improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants