Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MLLIB] SPARK-5491 (ex SPARK-1473): Chi-square feature selection #1484

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.mllib.feature

import org.apache.spark.annotation.Experimental
import org.apache.spark.mllib.linalg.{DenseVector, SparseVector, Vectors, Vector}
import org.apache.spark.mllib.regression.LabeledPoint
import org.apache.spark.mllib.stat.Statistics
import org.apache.spark.rdd.RDD

import scala.collection.mutable.ArrayBuilder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

organize imports (If you use idea intellij, there is a useful plugin: https://plugins.jetbrains.com/plugin/7350)


/**
* :: Experimental ::
* Chi Squared selector model.
*
* @param indices list of indices to select (filter). Must be ordered asc
*/
@Experimental
class ChiSqSelectorModel private[mllib] (indices: Array[Int]) extends VectorTransformer {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove one space before private. It should be okay to expose the constructor as well as indices. Otherwise, user won't be able to save/load a model and there is no easy way to obtain indices. We need to check the ordering of indices in the constructor then. For the name, is it clear that indices means selected feature indices under this context? selectedFeatures may sound better, which we can put under a feature selector trait.

/**
* Applies transformation on a vector.
*
* @param vector vector to be transformed.
* @return transformed vector.
*/
override def transform(vector: Vector): Vector = {
compress(vector, indices)
}

/**
* Returns a vector with features filtered.
* Preserves the order of filtered features the same as their indices are stored.
* Might be moved to Vector as .slice
* @param features vector
* @param filterIndices indices of features to filter, must be ordered asc
*/
private def compress(features: Vector, filterIndices: Array[Int]): Vector = {
features match {
case SparseVector(size, indices, values) =>
val newSize = filterIndices.length
val newValues = new ArrayBuilder.ofDouble
val newIndices = new ArrayBuilder.ofInt
var i: Int = 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove : Int (because this is a local var and it is clearly an Int)

var j: Int = 0
while(i < indices.length && j < filterIndices.length) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after while

if(indices(i) == filterIndices(j)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after if

newIndices += j
newValues += values(i)
j += 1
i += 1
} else {
if(indices(i) > filterIndices(j)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after if. For performance, we can store indices(i) and filterIndices(j) to avoid look up twice

j += 1
} else {
i += 1
}
}
}
/** Sparse representation might be ineffective if (newSize ~= newValues.size) */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Use a normal comment style // TODO: Sparse .... /** ... */ is for JavaDoc.

Vectors.sparse(newSize, newIndices.result(), newValues.result())
case DenseVector(values) =>
val values = features.toArray
Vectors.dense(filterIndices.map(i => values(i)))
case other =>
throw new UnsupportedOperationException(
s"Only sparse and dense vectors are supported but got ${other.getClass}.")
}
}
}

/**
* :: Experimental ::
* Creates a ChiSquared feature selector.
* @param numTopFeatures number of features that selector will select
* (ordered by statistic value descending)
*/
@Experimental
class ChiSqSelector (val numTopFeatures: Int) {

/**
* Returns a ChiSquared feature selector.
*
* @param data data used to compute the Chi Squared statistic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/
def fit(data: RDD[LabeledPoint]): ChiSqSelectorModel = {
val indices = Statistics.chiSqTest(data)
.zipWithIndex.sortBy { case(res, _) => -res.statistic }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after case

.take(numTopFeatures)
.map{ case(_, indices) => indices }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space before { and after case

.sorted
new ChiSqSelectorModel(indices)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.spark.mllib.feature

import org.scalatest.FunSuite

import org.apache.spark.mllib.linalg.Vectors
import org.apache.spark.mllib.regression.LabeledPoint
import org.apache.spark.mllib.util.MLlibTestSparkContext

class ChiSqSelectorSuite extends FunSuite with MLlibTestSparkContext {

/*
* Contingency tables
* feature0 = {8.0, 0.0}
* class 0 1 2
* 8.0||1|0|1|
* 0.0||0|2|0|
*
* feature1 = {7.0, 9.0}
* class 0 1 2
* 7.0||1|0|0|
* 9.0||0|2|1|
*
* feature2 = {0.0, 6.0, 8.0, 5.0}
* class 0 1 2
* 0.0||1|0|0|
* 6.0||0|1|0|
* 8.0||0|1|0|
* 5.0||0|0|1|
*
* Use chi-squared calculator from Internet
*/

test("ChiSqSelector transform test (sparse & dense vector)") {
val labeledDiscreteData = sc.parallelize(
Seq(new LabeledPoint(0.0, Vectors.sparse(3, Array((0, 8.0), (1, 7.0)))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove new. LabeledPoint is a case class.

new LabeledPoint(1.0, Vectors.sparse(3, Array((1, 9.0), (2, 6.0)))),
new LabeledPoint(1.0, Vectors.dense(Array(0.0, 9.0, 8.0))),
new LabeledPoint(2.0, Vectors.dense(Array(8.0, 9.0, 5.0)))
), 2)
val preFilteredData =
Set(new LabeledPoint(0.0, Vectors.dense(Array(0.0))),
new LabeledPoint(1.0, Vectors.dense(Array(6.0))),
new LabeledPoint(1.0, Vectors.dense(Array(8.0))),
new LabeledPoint(2.0, Vectors.dense(Array(5.0)))
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: In Spark, we don't make a new line for the ending ).

val model = new ChiSqSelector(1).fit(labeledDiscreteData)
val filteredData = labeledDiscreteData.map { lp =>
new LabeledPoint(lp.label, model.transform(lp.features))
}.collect().toSet
assert(filteredData == preFilteredData)
}
}