Skip to content

Preserve dtype for NumPy array-protocol inputs - #8432

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
viclafargue:preserve-dtype-numpy-array-protocol
Aug 4, 2026
Merged

Preserve dtype for NumPy array-protocol inputs#8432
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
viclafargue:preserve-dtype-numpy-array-protocol

Conversation

@viclafargue

Copy link
Copy Markdown
Contributor

Closes #8430.

Problem : The issue seems to be unrelated to the GB300 platform. Scikit-learn’s array-like compatibility checks wrap float64 data in an object without a direct dtype attribute. cuML consequently defaulted the wrapped input to float32, producing different results from the original float64 input.

Solution : Normalize objects implementing array before dtype selection to preserve their represented dtype, and remove the stale xfails.

@viclafargue
viclafargue requested a review from a team as a code owner August 3, 2026 09:20
@viclafargue
viclafargue requested a review from betatim August 3, 2026 09:20
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Aug 3, 2026
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

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: b6574e1a-f0e2-424d-889f-7dba1138f273

📥 Commits

Reviewing files that changed from the base of the PR and between bc14f49 and c7ab6d3.

📒 Files selected for processing (3)
  • python/cuml/cuml/internals/validation.py
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
  • python/cuml/tests/test_validation.py
💤 Files with no reviewable changes (1)
  • python/cuml/tests/test_validation.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • python/cuml/cuml/internals/validation.py
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved array validation for array-like inputs that provide __array__, preserving their original data type.
    • Enhanced compatibility across supported memory types and allowed data types.
    • Updated compatibility reporting for estimator and dimensionality-reduction checks to reflect current behavior.
  • Tests

    • Added coverage for dtype preservation across device, host, and unspecified memory.
    • Removed obsolete expected-failure markers for non-array input checks.

Walkthrough

check_array now infers dtypes from objects implementing __array__. Tests cover float32 and float64 inputs across memory types. The sklearn compatibility xfail entries and explanations now match current failures.

Changes

Array validation and compatibility

Layer / File(s) Summary
Array protocol dtype inference
python/cuml/cuml/internals/validation.py, python/cuml/tests/test_validation.py
check_array materializes array-protocol objects before dtype selection. Tests verify float32 and float64 preservation across supported memory types.
Non-array compatibility checks
python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml, python/cuml/tests/test_sklearn_compatibility.py
The xfail list replaces four estimator entries with the RandomForestRegressor non-array check and removes its duplicate entry. Remaining explanations describe repeated-fit prediction differences and inconsistent fit_transform and transform outputs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • rapidsai/cuml#8366: Updates overlapping sklearn compatibility xfail handling in the same test files.

Suggested reviewers: csadorf

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% 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 clearly identifies the primary change: preserving dtype for NumPy array-protocol inputs.
Description check ✅ Passed The description explains the dtype bug, its impact on compatibility checks, and the implemented fix.
Linked Issues check ✅ Passed The changes address issue #8430 by preserving wrapped input dtypes and updating related compatibility test expectations.
Out of Scope Changes check ✅ Passed The code, tests, xfail updates, and SPDX attribution change remain related to the dtype compatibility fix.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@betatim betatim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like a reasonable fix.

Comment thread python/cuml/tests/test_sklearn_compatibility.py
Comment thread python/cuml/tests/test_sklearn_compatibility.py
@viclafargue viclafargue added bug Something isn't working non-breaking Non-breaking change labels Aug 3, 2026
@jcrist
jcrist self-requested a review August 3, 2026 16:17

@jcrist jcrist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fix looks good to me! A bit disappointing that this didn't fix the issues in test_sklearn_compatibility.py though.

@viclafargue

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 3d02990 into NVIDIA:main Aug 4, 2026
178 of 181 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Cython / Python Cython or Python issue non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Failing tests on GB300

5 participants