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

Fix BinarySearchBin() argument types #7026

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

tpboudreau
Copy link
Contributor

@tpboudreau tpboudreau commented Jun 6, 2021

Fix for bug described in #7025.

With this fix, training as described in #7025 with tree_method=gpu_hist produces results comparable to tree_method=hist.

Dev/Test environment:
Linux: Linux acfa9079b903 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
CUDA: NVIDIA-SMI 460.80, Driver Version: 460.80, CUDA Version: 11.2

@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2021

Codecov Report

Merging #7026 (76d908e) into master (7beb2f7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7026   +/-   ##
=======================================
  Coverage   81.72%   81.72%           
=======================================
  Files          13       13           
  Lines        3917     3917           
=======================================
  Hits         3201     3201           
  Misses        716      716           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7beb2f7...76d908e. Read the comment docs.

Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

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

The change looks good to me. I will start setting up some testing environments for single GPU with inputs larger than 1^32.

@trivialfis trivialfis merged commit bd2ca54 into dmlc:master Jun 8, 2021
@tpboudreau
Copy link
Contributor Author

Thanks @trivialfis for reviewing this so quickly!

@trivialfis
Copy link
Member

Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants