Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/14597#discussion_r75289596
--- Diff:
mllib/src/main/scala/org/apache/spark/mllib/feature/ChiSqSelector.scala ---
@@ -189,11 +228,35 @@ 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
+ chiSqTestResult = Statistics.chiSqTest(data)
+ selectorType match {
+ case ChiSqSelectorType.KBest => selectKBest(numTopFeatures)
+ case ChiSqSelectorType.Percentile => selectPercentile(percentile)
+ case ChiSqSelectorType.Fpr => selectFpr(alpha)
+ case _ => throw new Exception("Unknown ChiSqSelector Type")
+ }
+ }
+
+ @Since("2.1.0")
+ def selectKBest(value: Int): ChiSqSelectorModel = {
--- End diff --
Yeah I thought the idea is that the model would be fixed, without
parameters, though it would have all the information internally (the chi
squared results) to select or re-select features to filter in based on
different logic. And that the selector would expose the parameters. I'm not
against your approach here just thought that's what you were saying would be
inconsistent. I suppose you're right that the model would have to mutate its
state to compute different sets of indices and that introduces a complication.
---
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]
