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 updates in random forest model using GOSS data sample strategy #6017

Merged
merged 11 commits into from Sep 5, 2023

Conversation

mjmckp
Copy link
Contributor

@mjmckp mjmckp commented Aug 2, 2023

1) Value of data_sample_strategy was not written out in Config::SaveMembersToString()
2) GOSSStrategy->Bagging may modify value of bag_data_cnt_ during training, which may mean tmp_grad_ and tmp_hess_ need resizing in RF::TrainOneIter. Without this change, an exception is thrown during training at line 138 of rf.hpp.

1) Value of data_sample_strategy was not written out in Config::SaveMembersToString()
2) GOSSStrategy->Bagging may modify value of bag_data_cnt_ during training, which may mean tmp_grad_ and tmp_hess_ need resizing in RF::TrainOneIter
src/io/config_auto.cpp Outdated Show resolved Hide resolved
This is required by LightGBMNet
@mjmckp mjmckp requested a review from jameslamb August 17, 2023 23:14
@mjmckp
Copy link
Contributor Author

mjmckp commented Aug 24, 2023

@jameslamb Are we ready to merge this now please?

@jameslamb
Copy link
Collaborator

Are we ready to merge this now please?

I've just merged master into this, and will merge it if/when it builds successfully.

In the future, you can reduce the time it takes to get changes reviewed and merged here by doing the following in pull requests:

  • clearly describing why changes are being made, not just what you're changing (ideally with a minimal reproducible example and/or new tests demonstrating the change in behavior)
  • linking directly to prior conversations and/or any relevant code, to help reviewers get context on exactly why you're proposing a set of changes
  • putting up separate PRs for unrelated concerns (in this example, updates in random forest model and changes to the model file could have been separated)

Thanks as always for your help with LightGBM!

@jameslamb jameslamb changed the title Fixes for GOSS data sample strategy Fix updates in random forest model using GOSS data sample strategy Sep 4, 2023
@jameslamb jameslamb merged commit 8203306 into microsoft:master Sep 5, 2023
41 checks passed
Copy link

github-actions bot commented Dec 6, 2023

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants