Skip to content

Simplify dask serializers - #8443

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
jcrist:simplify-dask-comms
Aug 4, 2026
Merged

Simplify dask serializers#8443
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
jcrist:simplify-dask-comms

Conversation

@jcrist

@jcrist jcrist commented Aug 4, 2026

Copy link
Copy Markdown
Member
  • Removes serializer override for MultinomialNB, this is unnecessary
  • Simplifies the registration of serializers for other types, and improves the comments to better state why overrides are needed.

This is a small followup to #8385. I noticed in that PR that some of the code here could be simplified/was no longer necessary.

This should have no runtime effects (the same effective code should run before and after), it's just a small code cleanup.

- Removes serializer override for `MultinomialNB`, this is unnecessary
- Simplifies the registration of serializers for other types, and
  improves the comments to better state why overrides are needed.
@jcrist jcrist self-assigned this Aug 4, 2026
@jcrist
jcrist requested a review from a team as a code owner August 4, 2026 00:05
@jcrist
jcrist requested a review from divyegala August 4, 2026 00:05
@jcrist jcrist added Dask / cuml.dask Issue/PR related to Python level dask or cuml.dask features. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 4, 2026
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Aug 4, 2026
@coderabbitai

coderabbitai Bot commented Aug 4, 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: a4bf69f5-350a-4615-9c1f-66ee46cbfb4d

📥 Commits

Reviewing files that changed from the base of the PR and between 0277bdf and 73cdfbc.

📒 Files selected for processing (1)
  • python/cuml/cuml/comm/serialize.py

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Streamlined internal serialization registration across supported distributed and GPU workflows.
    • Updated random forest models to use pickle-based serialization.
    • Removed obsolete serializer registrations without changing the public API.

Walkthrough

The serialization module replaces individual registrations with loop-based cuml.Base registration for Dask and CUDA serializers. Random forest classifier and regressor classes use explicit pickle overrides. MultinomialNB generic registrations are removed.

Changes

Serialization registration

Layer / File(s) Summary
Consolidate serializer registrations
python/cuml/cuml/comm/serialize.py
A loop registers Dask and CUDA serializers for cuml.Base. Random forest classifier and regressor classes explicitly use pickle serialization. Previous individual functions and MultinomialNB registrations are removed.

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

Suggested reviewers: divyegala

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: simplifying Dask serializer registration.
Description check ✅ Passed The description accurately explains the serializer cleanup, removed override, clarified comments, and intended lack of runtime effects.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Thanks! LGTM

@jcrist

jcrist commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit eecfb7c into NVIDIA:main Aug 4, 2026
101 checks passed
@jcrist
jcrist deleted the simplify-dask-comms branch August 4, 2026 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Cython / Python Cython or Python issue Dask / cuml.dask Issue/PR related to Python level dask or cuml.dask features. 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.

4 participants