Skip to content

Update Dask RF to use the new distributed algo - #8466

Open
chyunsu3 wants to merge 4 commits into
NVIDIA:mainfrom
chyunsu3:distributed_rf_dask
Open

Update Dask RF to use the new distributed algo#8466
chyunsu3 wants to merge 4 commits into
NVIDIA:mainfrom
chyunsu3:distributed_rf_dask

Conversation

@chyunsu3

@chyunsu3 chyunsu3 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor
  • Wire Dask RF training to RAFT communicators
  • Retain one fitted worker model as the canonical model for prediction and get_combined_model().
  • Remove obsolete components: per-worker n_estimator, ignore_empty_partitions, partial inference, etc
  • Update terminology: FIL -> nvForest
  • Use global n_rows when constraining certain hyperparameters
  • Update pytests

@copy-pr-bot

copy-pr-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@chyunsu3
chyunsu3 marked this pull request as ready for review August 11, 2026 17:08
@chyunsu3
chyunsu3 requested a review from a team as a code owner August 11, 2026 17:08
@chyunsu3
chyunsu3 requested a review from viclafargue August 11, 2026 17:08
@josephine-wolf-oberholtzer josephine-wolf-oberholtzer added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 11, 2026
@chyunsu3
chyunsu3 requested a review from RAMitchell August 12, 2026 02:35

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

Cleaned up code looks great!

We shouldn't outright remove parameters. Instead warn the parameter is no longer used and flag deprection.

I think only 1 stream ever gets used in the distributed version.

comms.init() is outside the try/finally

@chyunsu3

chyunsu3 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

only 1 stream ever gets used in the distributed version.

Should the distribute algo use multiple streams? The distributed algo uses a single stream only. Perhaps it's a good idea to throw a warning when n_streams>1

@RAMitchell

Copy link
Copy Markdown
Contributor

@chyunsu3 yes the parameter is currently unused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CMake CUDA/C++ 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.

3 participants