Github user MLnick commented on the issue:
https://github.com/apache/spark/pull/15593
@dbtsai I understand wanting to make things clear & easy to understand for
current and future developers. Of course I would like the same.
However, it's currently not clear to me how (or if) your proposal above
would work to achieve that goal. Concrete example code would help a lot here (I
am probably missing something in my interpretation).
To be honest, even the current code is difficult to understand at first (or
even third!) glance for a developer unfamiliar with the implementation (perhaps
we can work to improve that, perhaps it can't be improved that much given we
have to work with arrays, low-level performance and the Breeze optimizers). I
don't think the col-major layout during training changes this situation
significantly over row-major. As long as it is clearly and correctly documented
I think it is ok (if the doc comments added here need improvement we can make
changes).
(By the way, the row- or col-major layout shouldn't make a difference for
BLoR, and indeed the BLoR code is not really touched. The indexing may look
different but is in practice the same).
I've worked through the impl in the PR and to me the changes look good. The
tests are pretty comprehensive and are a good check on things so we can be
comfortable. This is an important performance fix that I think must be in 2.1.
If you can show a way to achieve the same but have more clarity &
simplicity, then I'm all for that, but I think that can be done in a separate
JIRA (even after 2.1 if need be).
Let me know if you disagree.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
