Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved multi-node multi-GPU random forests. #4238

Merged
merged 8 commits into from
Mar 12, 2019

Conversation

canonizer
Copy link
Contributor

  • removed rabit::Broadcast() from each invocation of column sampling
  • instead, syncing the PRNG seed when a ColumnSampler() object is constructed
  • this makes non-trivial column sampling significantly faster in the distributed case
  • refactored distributed GPU tests
  • added distributed random forests tests

- removed rabit::Broadcast() from each invocation of column sampling
- instead, syncing the PRNG seed when a ColumnSampler() object is constructed
- this makes non-trivial column sampling significantly faster in the distributed case
- refactored distributed GPU tests
- added distributed random forests tests
Copy link
Member

@RAMitchell RAMitchell left a comment

Choose a reason for hiding this comment

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

I don't think it's necessary for us to have a separate folder distributed-gpu, these tests can just go in the distributed folder. I have been meaning to do the same with python tests. This folder structure was only an artefact of running tests using nose, which we don't use any more.

# Notify the tracker all training has been successful
# This is only needed in distributed training.
xgb.rabit.finalize()
import distributed_gpu as dgpu
Copy link
Member

Choose a reason for hiding this comment

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

I think these python files can be combined into a single file using command line arguments to configure the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@RAMitchell RAMitchell merged commit b833b64 into dmlc:master Mar 12, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 11, 2019
@mtjrider mtjrider deleted the fea-ext-mnmg-rf branch August 19, 2019 17:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants