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
[MRG] Reuse grower and splitter memory #88
Conversation
@@ -86,7 +86,10 @@ def load_data(): | |||
n_iter_no_change=None, | |||
random_state=0, | |||
verbose=1) | |||
pygbm_model.fit(data_train, target_train) | |||
@profile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove this
Here is the output of a run of Higgs boson with
So basically there is no significant difference. I think it means that the memory used by the reused splitting context instance is very little compared to the memory used by the histograms stored on each node instance. Those linked nodes in the grower tree it-self cannot be reused very easily without significant refactoring as far as I know. |
Also note that according to the above chart, the memory usage of pygbm is not that bad even if it's fluctuating because of the garbage collections of the temporary histogram arrays. |
Thanks for the plots! Indeed it doesn't seem to help much, at least with this number of samples. I don't think histograms use much memory compared to the splitter though, according to my calculations: histograms for one iteration:
SplittingContext for one iteration (re-used):
That being said, LightGBM uses an LRU cache for the histograms. Maybe it would be helpful if @maartenbreddels could try this branch on the big dataset that used to cause a memory stress? Until then I'm in favor for closing the PR, and maybe resuscitate it eventually if needed. |
Indeed you are right about the relative sizes of the datastructures. But then I don't understand what is causing the fluctuations when running the benchmark on this PR (with the reset-able splitting context variant). |
Well maybe is just like you said, the |
Happy to test this branch out (maybe this week? can't guarantee), note that I was using OS X, maybe that mattters. |
Here is a possible fix for the accumulated memory usage of the stored histograms that proved very efficient in scikit-learn: scikit-learn/scikit-learn#18242 |
Also note that Higgs boson is not the best benchmark for memory efficiency: it has many samples and few features and a single binary target. So compared to the amount of computation, it allocates few histograms. |
Closes #81
Instead of creating a new grower and a new splitter for every tree, we reuse the existing objects and thus the allocated arrays (
ordered_gradients
,partition
, etc.)I didn't observe any significant speed improvement with:
but that might come with parallelization.
I haven't run any memory usage benchmark, I'm not sure if
memory_profiler
is really suited for this.@ogrisel if you're OK with this in general I'll fix the tests (
compare_lightgbm
is already passing) and I will parallelizeSplittingContext.reset()
.