Skip to content

Forward merge release/26.04 into main - #7936

Merged
jameslamb merged 5 commits into
NVIDIA:mainfrom
jcrist:main-merge-release/26.04
Mar 31, 2026
Merged

Forward merge release/26.04 into main#7936
jameslamb merged 5 commits into
NVIDIA:mainfrom
jcrist:main-merge-release/26.04

Conversation

@jcrist

@jcrist jcrist commented Mar 31, 2026

Copy link
Copy Markdown
Member

No description provided.

csadorf and others added 5 commits March 23, 2026 14:49
## Summary

The PR/issue project automation workflow set `OVERRIDE_ITEM: true` for PRs targeting `release/*`, so every `synchronize` run re-applied the Release field from the base branch name and overwrote manual values (e.g. after retargeting).

## Changes

- Set `OVERRIDE_ITEM: false` on `update-release` for all cases that run that job (no conditional).
- Drop the `override-item` job output and the `echo "override-item=…"` lines from the determine-version step.

Authors:
  - Simon Adorf (https://github.com/csadorf)

Approvers:
  - James Lamb (https://github.com/jameslamb)

URL: NVIDIA#7921
- Bump sklearn versions we test with.
- Remove no longer relevant non-numeric limitation.

Authors:
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - Tim Head (https://github.com/betatim)

URL: NVIDIA#7929
…VIDIA#7807)

Closes NVIDIA#7806

When there are more workers than data partitions, some workers end up with no data, causing hangs or crashes in collective operations that expect all participants to contribute rows.

The changes in this PR prune workers with zero-row partitions from `DistributedDataHandler` before initializing `NCCL` communicators.

Authors:
  - Victor Lafargue (https://github.com/viclafargue)
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - James Lamb (https://github.com/jameslamb)
  - Jim Crist-Harif (https://github.com/jcrist)

URL: NVIDIA#7807
We're seeing some test failures with the new openblas release on aarch64 (accuracy issues on comparison with sklearn's results, _not_ issues in cuml's execution). I suspect a regression in openblas.

For now we pin openblas on aarch64.

Authors:
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - James Lamb (https://github.com/jameslamb)

URL: NVIDIA#7933
@jcrist jcrist self-assigned this Mar 31, 2026
@jcrist
jcrist requested review from a team as code owners March 31, 2026 16:50
@github-actions github-actions Bot added conda conda issue Cython / Python Cython or Python issue labels Mar 31, 2026
@jcrist jcrist added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 31, 2026

@bdice bdice 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.

Make sure to /merge nosquash when merging this.

@coderabbitai

coderabbitai Bot commented Mar 31, 2026

Copy link
Copy Markdown
📝 Walkthrough

Summary by CodeRabbit

  • Documentation

    • Updated scikit-learn compatibility range for cuML accelerator to 1.5–1.8
    • Removed documentation about string array unsupported limitation
  • Bug Fixes

    • Improved handling of empty data partitions in distributed processing environments
  • Chores

    • Added libopenblas dependency constraint for aarch64 Conda environments

Walkthrough

This PR refactors distributed data handling in Dask workflows to cache per-worker partition row counts and gracefully handle empty partitions. It also updates CI automation, adds OpenBLAS version constraints to conda environments, updates scikit-learn compatibility documentation, and adds comprehensive tests for empty worker scenarios.

Changes

Cohort / File(s) Summary
CI Workflow Automation
.github/workflows/pr_issue_status_automation.yml
Removed the override-item output computation from get-release-version and hardcoded the downstream update-release workflow's OVERRIDE_ITEM input to false instead of deriving it conditionally.
Conda Environment Dependencies
conda/environments/all_cuda-129_arch-aarch64.yaml, conda/environments/all_cuda-131_arch-aarch64.yaml
Added libopenblas<=0.3.30 dependency constraint to both aarch64 CUDA conda environments.
Dependency Configuration
dependencies.yaml
Added conditional dependency matrix block to pin libopenblas<=0.3.30 for aarch64 conda test environments, placed before the default unconstrained packages matrix.
Documentation Updates
docs/source/cuml-accel/limitations.rst
Removed documentation about unsupported string array targets and updated scikit-learn compatibility range from 1.4–1.7 to 1.5–1.8.
Dask Distributed Data Handler Refactoring
python/cuml/cuml/dask/common/input_utils.py
Added _worker_sizes caching attribute and _fetch_worker_sizes() method to detect and prune empty workers. Refactored calculate_parts_to_sizes() to use cached worker sizes and removed unused ranks parameter. Emits UserWarning when workers with zero rows are detected.
Dask Nearest Neighbors
python/cuml/cuml/dask/neighbors/nearest_neighbors.py
Removed explanatory comment about communicator clique worker membership requirements.
Test Additions
python/cuml/tests/dask/test_dask_base.py
Added helper function _make_dask_data_with_empty_worker and three unit tests validating DistributedDataHandler.create warning emission, worker filtering, and row accounting for empty partitions. Added integration test training LogisticRegression on mixed (real + empty) partitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

conda, Cython / Python, improvement, non-breaking, ci, bug

Suggested reviewers

  • viclafargue
  • csadorf
  • msarahan
🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author; the pull request lacks any documentation of changes or intent. Add a pull request description explaining the purpose of this forward merge and summarizing key changes being merged from the release branch.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies this as a forward merge from release/26.04 into main, which accurately describes the pull request's primary purpose.

✏️ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@python/cuml/cuml/dask/common/input_utils.py`:
- Around line 145-151: The warnings.warn call in input_utils.py (inside the code
path that logs empty_workers) lacks a stacklevel, causing Ruff B028 and pointing
the warning at the utility instead of the caller; update the warnings.warn
invocation (the one that constructs the message about empty_workers) to include
an explicit stacklevel parameter (e.g., stacklevel=2 or higher) so the warning
originates from the caller location.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 32cac1f0-f724-4434-8bfe-d1da7a44a502

📥 Commits

Reviewing files that changed from the base of the PR and between 5e954f7 and 54d3634.

📒 Files selected for processing (8)
  • .github/workflows/pr_issue_status_automation.yml
  • conda/environments/all_cuda-129_arch-aarch64.yaml
  • conda/environments/all_cuda-131_arch-aarch64.yaml
  • dependencies.yaml
  • docs/source/cuml-accel/limitations.rst
  • python/cuml/cuml/dask/common/input_utils.py
  • python/cuml/cuml/dask/neighbors/nearest_neighbors.py
  • python/cuml/tests/dask/test_dask_base.py
💤 Files with no reviewable changes (1)
  • python/cuml/cuml/dask/neighbors/nearest_neighbors.py

Comment thread python/cuml/cuml/dask/common/input_utils.py
@jcrist

jcrist commented Mar 31, 2026

Copy link
Copy Markdown
Member Author

/merge nosquash

@rapids-bot

rapids-bot Bot commented Mar 31, 2026

Copy link
Copy Markdown
Contributor

Commit history integrity check failed: not all commits from original PR #7923 appear to be present individually in this PR's history. This usually happens if commits were squashed during the manual resolution process. Please ensure all original commits are preserved individually. You can fix this and try the /merge nosquash command again.

@jameslamb
jameslamb merged commit 2539c61 into NVIDIA:main Mar 31, 2026
191 of 193 checks passed
@jcrist
jcrist deleted the main-merge-release/26.04 branch March 31, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conda conda issue Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants