Github user manishamde commented on the pull request:
https://github.com/apache/spark/pull/1582#issuecomment-50107957
@jkbradley Awesome!
A couple of quick thoughts:
+ I am not completely convinced about the strategy for1a (I was expecting
thin wrappers for regression and classification tree) but I guess that was
expected considering I am very familiar with the existing code. I will sleep
over it and get back. :-) To give a historical perspective, we had a similar
split implementations for regression and classification in the beginning that
we decided to combine into one. Perhaps, it's the right time to split them
again. @etrain was also hinting at that in the multiclass review.
+ I have ensemble RF and Boosting implementations close-to-ready which will
need major refactoring or rewriting from scratch considering the magnitude of
this PR. That's fine but we should try and get it accepted ASAP. I promise
prompt piecemeal reviews.
+ We should perform regression testing and compare with the 1.0 release.
---
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.
---
