Avoid copy on Refit #6478
Open
Avoid copy on Refit #6478
+31
−22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbourjau
force-pushed
the
avoid-copy-refit
branch
from
898acdb
to
aa3bcff
Compare
cbourjau
requested review from
guolinke,
jameslamb,
shiyu1994,
jmoralez and
borchero
as code owners
jameslamb
requested changes
Jun 12, 2024
There was a problem hiding this comment.
Thanks for your interest in LightGBM. It'll be at least a few days until someone's able to review this, as we're currently focused on finishing a release (#6439 (comment)).
Every CI run on this PR will require a maintainer manually approving it, because you've never contributed here before. Sorry for the inconvenience, but GitHub introduced this as a security measure a few years ago and we've decided to leave it enabled. We do occasionally receive malicious pull requests trying to use our CI resources 🙃 |
|
Thanks for the feedback! Some CI does appear to run, though, and some of it exhibited IO failures the first time around which I was trying to address 🤷 . |
borchero
requested changes
Jun 14, 2024
Co-authored-by: Oliver Borchert <[email protected]>
Co-authored-by: Oliver Borchert <[email protected]>
borchero
requested changes
Jun 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The C-API exposes
LGBM_BoosterRefitwhich receives the predicted leaf indices as a flat buffer that is laid out in the shape ofnrow x ncol. TheBoosting::RefitTreefunction, on the other hand, expects these indices as a nested vector object (std::vector<std::vector<int>>). Creating this nested object requires various additional allocations and amounts to an entire copy of the initial buffer doubling the memory requirements.This PR changes the API of
Boosting::RefitTreeto take a pointer to a flat buffer ofint32s, just like the C-API, thus avoiding the copy. This assumes that this part of the API is not regarded as stable. The C-API remains unchanged.