Skip to content

[SPARK-18434][ML] Add missing ParamValidations for ML algos #15881

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
zhengruifeng wants to merge 4 commits into from
Closed

[SPARK-18434][ML] Add missing ParamValidations for ML algos #15881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eecb4d7
create pr
zhengruifeng Nov 14, 2016
5ff61e6
create pr
zhengruifeng Nov 14, 2016
89c7f1a
update
zhengruifeng Nov 14, 2016
587fb9c
array->set; show supported options
zhengruifeng Nov 15, 2016
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
3 changes: 2 additions & 1 deletion mllib/src/main/scala/org/apache/spark/ml/feature/IDF.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ private[feature] trait IDFBase extends Params with HasInputCol with HasOutputCol
* @group param
*/
final val minDocFreq = new IntParam(
this, "minDocFreq", "minimum number of documents in which a term should appear for filtering")
this, "minDocFreq", "minimum number of documents in which a term should appear for filtering" +
" (>= 0)", ParamValidators.gtEq(0))

setDefault(minDocFreq -> 0)

Expand Down
3 changes: 2 additions & 1 deletion mllib/src/main/scala/org/apache/spark/ml/feature/PCA.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ private[feature] trait PCAParams extends Params with HasInputCol with HasOutputC
* The number of principal components.
* @group param
*/
final val k: IntParam = new IntParam(this, "k", "the number of principal components")
final val k: IntParam = new IntParam(this, "k", "the number of principal components (> 0)",
ParamValidators.gt(0))

/** @group getParam */
def getK: Int = $(k)
Expand Down
13 changes: 8 additions & 5 deletions mllib/src/main/scala/org/apache/spark/ml/feature/Word2Vec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ private[feature] trait Word2VecBase extends Params
* @group param
*/
final val vectorSize = new IntParam(
this, "vectorSize", "the dimension of codes after transforming from words")
this, "vectorSize", "the dimension of codes after transforming from words (> 0)",
ParamValidators.gt(0))
setDefault(vectorSize -> 100)

/** @group getParam */
Expand All @@ -55,7 +56,8 @@ private[feature] trait Word2VecBase extends Params
* @group expertParam
*/
final val windowSize = new IntParam(
this, "windowSize", "the window size (context words from [-window, window])")
this, "windowSize", "the window size (context words from [-window, window]) (> 0)",
ParamValidators.gt(0))
setDefault(windowSize -> 5)

/** @group expertGetParam */
Expand All @@ -67,7 +69,8 @@ private[feature] trait Word2VecBase extends Params
* @group param
*/
final val numPartitions = new IntParam(
this, "numPartitions", "number of partitions for sentences of words")
this, "numPartitions", "number of partitions for sentences of words (> 0)",
ParamValidators.gt(0))
setDefault(numPartitions -> 1)

/** @group getParam */
Expand All @@ -80,7 +83,7 @@ private[feature] trait Word2VecBase extends Params
* @group param
*/
final val minCount = new IntParam(this, "minCount", "the minimum number of times a token must " +
"appear to be included in the word2vec model's vocabulary")
"appear to be included in the word2vec model's vocabulary (>= 0)", ParamValidators.gtEq(0))
setDefault(minCount -> 5)

/** @group getParam */
Expand All @@ -95,7 +98,7 @@ private[feature] trait Word2VecBase extends Params
*/
final val maxSentenceLength = new IntParam(this, "maxSentenceLength", "Maximum length " +
"(in words) of each sentence in the input data. Any sentence longer than this threshold will " +
"be divided into chunks up to the size.")
"be divided into chunks up to the size (> 0)", ParamValidators.gt(0))
setDefault(maxSentenceLength -> 1000)

/** @group getParam */
Expand Down
3 changes: 2 additions & 1 deletion mllib/src/main/scala/org/apache/spark/ml/regression/IsotonicRegression.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ private[regression] trait IsotonicRegressionBase extends Params with HasFeatures
* @group param
*/
final val featureIndex: IntParam = new IntParam(this, "featureIndex",
"The index of the feature if featuresCol is a vector column, no effect otherwise.")
"The index of the feature if featuresCol is a vector column, no effect otherwise (>= 0)",
ParamValidators.gtEq(0))

/** @group getParam */
final def getFeatureIndex: Int = $(featureIndex)
Expand Down
6 changes: 5 additions & 1 deletion mllib/src/main/scala/org/apache/spark/ml/regression/LinearRegression.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,11 @@ class LinearRegression @Since("1.3.0") (@Since("1.3.0") override val uid: String
* @group setParam
*/
@Since("1.6.0")
def setSolver(value: String): this.type = set(solver, value)
def setSolver(value: String): this.type = {
require(Set("auto", "l-bfgs", "normal").contains(value),
s"Solver $value was not supported. Supported options: auto, l-bfgs, normal")
set(solver, value)
}
setDefault(solver -> "auto")

/**
Expand Down
4 changes: 3 additions & 1 deletion mllib/src/main/scala/org/apache/spark/ml/tree/treeParams.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,13 @@ private[ml] trait DecisionTreeParams extends PredictorParams

/**
* Minimum information gain for a split to be considered at a tree node.
* Should be >= 0.0.
* (default = 0.0)
* @group param
*/
final val minInfoGain: DoubleParam = new DoubleParam(this, "minInfoGain",
"Minimum information gain for a split to be considered at a tree node.")
"Minimum information gain for a split to be considered at a tree node.",
Copy link
Contributor

@hhbyyh hhbyyh Nov 15, 2016

Choose a reason for hiding this comment

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

Sorry to miss this. Perhaps add >=0 at the end to be consistent with others.

Copy link
Contributor Author

@zhengruifeng zhengruifeng Nov 15, 2016

Choose a reason for hiding this comment

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

👌， I will add it

Copy link
Contributor Author

@zhengruifeng zhengruifeng Nov 15, 2016

Choose a reason for hiding this comment

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

Oh, I think it's better to not add >=0 here, because all params in treeParams.scala don't add it.

ParamValidators.gtEq(0.0))

/**
* Maximum memory in MB allocated to histogram aggregation. If too small, then 1 node will be
Expand Down