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 calculation of number of bins in FindGroup #6019

Merged
merged 8 commits into from
Feb 20, 2024

Conversation

CVPaul
Copy link
Contributor

@CVPaul CVPaul commented Aug 4, 2023

Fixes #3339.

The calculation method for num_total_bin used during Exclusive Feature Bundling

LightGBM/src/io/dataset.cpp

Line 134 in 665c473

(bin_mappers[fidx]->GetDefaultBin() == 0 ? -1 : 0);
doesn't align completely with the way num_total_bin is calculated during the creation of a FeatureGroup
LightGBM/include/LightGBM/feature_group.h

Line 68 in 665c473

if (bin_mappers_[i]->GetMostFreqBin() == 0) {
As a result, the max_bin_per_group (=256) is working during Bundling, but it is not working when creating the FeatureGroup. When I replaced the GetDefaultBin() at dataset.cpp#L134 with GetMostFreqBin(), the issue was resolved.

@CVPaul
Copy link
Contributor Author

CVPaul commented Aug 4, 2023

@microsoft-github-policy-service agree

@jameslamb
Copy link
Collaborator

@shiyu1994 could you please edit the title of this to make it more informative in the release notes? I would, but I don't really understand the effect of this PR.

@shiyu1994 shiyu1994 changed the title Solve issue 3339 Fix calculation of number of bins in FindGroup Sep 12, 2023
@Borda
Copy link

Borda commented Feb 7, 2024

this would be great to have in any release soon, please 🙏

@CVPaul CVPaul requested a review from borchero as a code owner February 8, 2024 16:32
@fillipemadureira
Copy link

this would be great to have in any release soon, please 🙏

I've just come across this issue. No release with this solution yet??

@karta282950
Copy link

I also meet same problem. When release this version?

@borchero
Copy link
Collaborator

Unfortunately, we are currently battling CI issues (cf. #6425). Once we got our GPU runners working again, we are planning to prepare another release -- this will likely happen in the early part of May.

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

Successfully merging this pull request may close these issues.

[Bug] LightGBMError: bin size 257 cannot run on GPU
8 participants