[SPARK-18471][MLLIB] Fix huge vectors of zero send in closure in L-BFGS #16078
Conversation
Introduced util tTreeAggregatoreWithZeroGenerator to avoid sending huge zero vector in L-BFGS or similar agregation, as only the size of the zero value to be generated is captured in the closure.
|
Can one of the admins verify this patch? |
|
This is getting a bit out of hand, with 6 pull requests now. To be clear, I do not think we should merge this change. It's not necessary to address the problem. |
|
It is necessary to address it in L-BFGS at least. We propose a solution in core which can be legitimately rejected as not relevant for core. And two solutions in MLlib, one provide for a reusable util to aggregate large vectors, the over one is a specific hack in L-BFGS. At least one of them deserves to be worked on I believe a Sparse vectors are not an option for performance reasons. |
|
In order to reduce the mess around multiple PRs I'll close this one and rebase the change in #16037 as you requested. What is the right way,convenient for you, to share a proposal without creating a PR btw ? |
|
@AnthonyTruchet I think in this case it was just confusing to have many PRs opened against the issue. One option is to either adjust the existing PR with changes (so that only one PR is open). You can mark the PR as "[WIP]" to indicate it's somewhat up for discussion still. An even better approach is to first discuss on the JIRA ticket and try to settle on an accepted solution, then open the PR. This is especially important if there is a proposed change to APIs or new core methods etc. |
4 participants
What changes were proposed in this pull request?
Introduced util TreeAggregatorWithZeroGenerator and used it to avoid
sending huge zero vector in L-BFGS or similar aggregation.
How was this patch tested?
Run custom L-BGFS using this aggregator instead of treeAggregate with significantly increased performance and stability.