Skip to content

[SPARK-20501] [ML] ML 2.2 QA: New Scala APIs, docs #17934

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.

Sign up for GitHub

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

Closed
yanboliang wants to merge 4 commits into from
Closed

[SPARK-20501] [ML] ML 2.2 QA: New Scala APIs, docs #17934

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
427295e
ML 2.2 QA: New Scala APIs, docs
yanboliang May 10, 2017
2aab9ba
Fix typo.
yanboliang May 10, 2017
65f22a0
Fix LinearSVC threshold.
yanboliang May 10, 2017
cef372b
Revert the change for StringIndexerModel.
yanboliang May 16, 2017
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
6 changes: 3 additions & 3 deletions mllib/src/main/scala/org/apache/spark/ml/classification/LinearSVC.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ private[classification] trait LinearSVCParams extends ClassifierParams with HasR
* Linear SVM Classifier</a>
*
* This binary classifier optimizes the Hinge Loss using the OWLQN optimizer.
* Only supports L2 regularization currently.

@felixcheung felixcheung May 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this something should be mentioned in R too?

@yanboliang yanboliang May 11, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@felixcheung Yeah, it should be. However, since SparkR exposed more MLlib APIs during 2.2 releasing cycle, I'm preparing a separate PR to audit new SparkR MLlib APIs which would include this update. Thanks.

*
*/
@Since("2.2.0")
Expand Down Expand Up @@ -131,7 +132,6 @@ class LinearSVC @Since("2.2.0") (
*/
@Since("2.2.0")
def setThreshold(value: Double): this.type = set(threshold, value)
setDefault(threshold -> 0.0)

@MLnick MLnick May 10, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why remove this?

@yanboliang yanboliang May 10, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Typo, reverted, thanks.


/**
* Suggested depth for treeAggregate (greater than or equal to 2).
Expand All @@ -148,7 +148,7 @@ class LinearSVC @Since("2.2.0") (
@Since("2.2.0")
override def copy(extra: ParamMap): LinearSVC = defaultCopy(extra)

override protected[classification] def train(dataset: Dataset[_]): LinearSVCModel = {
override protected def train(dataset: Dataset[_]): LinearSVCModel = {
val w = if (!isDefined(weightCol) || $(weightCol).isEmpty) lit(1.0) else col($(weightCol))
val instances: RDD[Instance] =
dataset.select(col($(labelCol)), w, col($(featuresCol))).rdd.map {
Expand Down Expand Up @@ -264,7 +264,7 @@ object LinearSVC extends DefaultParamsReadable[LinearSVC] {

/**
* :: Experimental ::
* SVM Model trained by [[LinearSVC]]
* Linear SVM Model trained by [[LinearSVC]]
*/
@Since("2.2.0")
@Experimental
Expand Down
8 changes: 6 additions & 2 deletions mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,12 @@ private[classification] trait LogisticRegressionParams extends ProbabilisticClas
}

/**
* Logistic regression. Supports multinomial logistic (softmax) regression and binomial logistic
* regression.
* Logistic regression. Supports:
* - Multinomial logistic (softmax) regression.
* - Binomial logistic regression.
*
* This class supports fitting traditional logistic regression model by LBFGS/OWLQN and
* bound (box) constrained logistic regression model by LBFGSB.
*/
@Since("1.2.0")
class LogisticRegression @Since("1.2.0") (
Expand Down
12 changes: 7 additions & 5 deletions mllib/src/main/scala/org/apache/spark/ml/feature/Imputer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ private[feature] trait ImputerParams extends Params with HasInputCols {
* computing median, DataFrameStatFunctions.approxQuantile is used with a relative error of 0.001.
*/
@Experimental
class Imputer @Since("2.2.0")(override val uid: String)
@Since("2.2.0")
class Imputer @Since("2.2.0") (@Since("2.2.0") override val uid: String)
extends Estimator[ImputerModel] with ImputerParams with DefaultParamsWritable {

@Since("2.2.0")
Expand Down Expand Up @@ -165,8 +166,8 @@ class Imputer @Since("2.2.0")(override val uid: String)
object Imputer extends DefaultParamsReadable[Imputer] {

/** strategy names that Imputer currently supports. */
private[ml] val mean = "mean"
private[ml] val median = "median"
private[feature] val mean = "mean"
private[feature] val median = "median"

@Since("2.2.0")
override def load(path: String): Imputer = super.load(path)
Expand All @@ -180,9 +181,10 @@ object Imputer extends DefaultParamsReadable[Imputer] {
* which are used to replace the missing values in the input DataFrame.
*/
@Experimental
@Since("2.2.0")
class ImputerModel private[ml](

@MLnick MLnick May 10, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: should be a space between private[ml] and the (

override val uid: String,
val surrogateDF: DataFrame)
@Since("2.2.0") override val uid: String,
@Since("2.2.0") val surrogateDF: DataFrame)
extends Model[ImputerModel] with ImputerParams with MLWritable {

import ImputerModel._
Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/feature/StringIndexer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ object StringIndexer extends DefaultParamsReadable[StringIndexer] {
* This is a temporary fix for the case when target labels do not exist during prediction.
*/
@Since("1.4.0")
class StringIndexerModel (
class StringIndexerModel private[ml] (

@MLnick MLnick May 10, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, making this private now will break user code if it is used, no? It's unfortunate that it wasn't private before, but do we want to do this?

@yanboliang yanboliang May 10, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We make this public by mistake, so I think we should fix this bug. AFAIK, this is the only model whose constructor is public, so there is little chance to break user code. And I think it makes sense to break user code by fixing bug. Thanks.

@jkbradley jkbradley May 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought this was made public on purpose. StringIndexerModel is one case where it makes sense for users to create it manually (if they already have a list of the possible Strings).

@yanboliang yanboliang May 16, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, I reverted back. Thanks.

@Since("1.4.0") override val uid: String,
@Since("1.5.0") val labels: Array[String])
extends Model[StringIndexerModel] with StringIndexerBase with MLWritable {
Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/fpm/FPGrowth.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ object FPGrowth extends DefaultParamsReadable[FPGrowth] {
@Experimental
class FPGrowthModel private[ml] (
@Since("2.2.0") override val uid: String,
@transient val freqItemsets: DataFrame)
@Since("2.2.0") @transient val freqItemsets: DataFrame)
extends Model[FPGrowthModel] with FPGrowthParams with MLWritable {

/** @group setParam */
Expand Down
2 changes: 1 addition & 1 deletion mllib/src/main/scala/org/apache/spark/ml/stat/Correlation.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import org.apache.spark.sql.{DataFrame, Dataset, Row}
import org.apache.spark.sql.types.{StructField, StructType}

/**
* API for correlation functions in MLlib, compatible with Dataframes and Datasets.
* API for correlation functions in MLlib, compatible with DataFrames and Datasets.
*
* The functions in this package generalize the functions in [[org.apache.spark.sql.Dataset#stat]]
* to spark.ml's Vector types.
Expand Down
4 changes: 2 additions & 2 deletions mllib/src/test/scala/org/apache/spark/ml/classification/LinearSVCSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class LinearSVCSuite extends SparkFunSuite with MLlibTestSparkContext with Defau
assert(lsvc.getTol === 1E-6)
assert(lsvc.getStandardization)
assert(!lsvc.isDefined(lsvc.weightCol))
assert(lsvc.getThreshold === 0.0)
assert(lsvc.getThreshold === 0.5)
assert(lsvc.getAggregationDepth === 2)
assert(lsvc.getLabelCol === "label")
assert(lsvc.getFeaturesCol === "features")
Expand All @@ -116,7 +116,7 @@ class LinearSVCSuite extends SparkFunSuite with MLlibTestSparkContext with Defau
model.transform(smallBinaryDataset)
.select("label", "prediction", "rawPrediction")
.collect()
assert(model.getThreshold === 0.0)
assert(model.getThreshold === 0.5)
assert(model.getFeaturesCol === "features")
assert(model.getPredictionCol === "prediction")
assert(model.getRawPredictionCol === "rawPrediction")
Expand Down
1 change: 1 addition & 0 deletions python/pyspark/ml/classification.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class LinearSVC(JavaEstimator, HasFeaturesCol, HasLabelCol, HasPredictionCol, Ha
`Linear SVM Classifier <https://en.wikipedia.org/wiki/Support_vector_machine#Linear_SVM>`_

This binary classifier optimizes the Hinge Loss using the OWLQN optimizer.
Only supports L2 regularization currently.

>>> from pyspark.sql import Row
>>> from pyspark.ml.linalg import Vectors
Expand Down