Github user chouqin commented on the pull request:

    https://github.com/apache/spark/pull/2780#issuecomment-59007084
  
    @manishamde thanks for your comments. I will adjust my code after #2785  
gets merged.
    
    As for performance, yes, this is slower than the current implementation, 
but I think this performance loss is lot very important, in that:
    
    1. It is a O(n) operation, where n is the size of `featureSamples`, 
compared to the sort operation(O(nlog(n))), 
    2. This function is called only once for each continuous feature, and It is 
done before the training step.
    It should not be a bottleneck because we can view it as a pre-process step.
    
    I am not very sure how much gain in accuracy this change will 
give(@jkbradley, are there any references to do this change). If it gives 
sufficient accuracy gain, 
    then this performance loss can be ignored.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to