Github user mpjlu commented on a diff in the pull request:
https://github.com/apache/spark/pull/14597#discussion_r76059026
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/feature/ChiSqSelector.scala ---
@@ -189,11 +232,21 @@ class ChiSqSelector @Since("1.3.0") (
*/
@Since("1.3.0")
def fit(data: RDD[LabeledPoint]): ChiSqSelectorModel = {
- val indices = Statistics.chiSqTest(data)
- .zipWithIndex.sortBy { case (res, _) => -res.statistic }
- .take(numTopFeatures)
- .map { case (_, indices) => indices }
- .sorted
+ val chiSqTestResult = Statistics.chiSqTest(data)
+ val features = selectorType match {
+ case ChiSqSelectorType.KBest => chiSqTestResult
+ .zipWithIndex.sortBy { case (res, _) => -res.statistic }
--- End diff --
Hi @srowen , do you mean we should move the sort from the model to here?
There are two sort here.
One is sort the indices, which we have been moved from here to the model.
The second is sort the features, because the KBest, and Percentile is
selection by the order of res.statistic, so we sort the features here. FPR is
selection by res.p-value > alpha, so don't need sort here.
---
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
