Skip to content

[SPARK-20351] [ML] Add trait hasTrainingSummary to replace the duplicate code #17654

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

[SPARK-20351] [ML] Add trait hasTrainingSummary to replace the duplicate code #17654

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3bca3b1
has summary trait
YY-OnCall Apr 16, 2017
ffbdb6e
resolve conflict
YY-OnCall Dec 13, 2018
4b660e3
Merge remote-tracking branch 'upstream/master' into hassummary
YY-OnCall Dec 13, 2018
0008b89
Merge remote-tracking branch 'upstream/master' into hassummary
YY-OnCall Dec 14, 2018
ecef1d0
remove unnecessary setter
YY-OnCall Dec 14, 2018
79ae7fe
fix comment and style
YY-OnCall Dec 15, 2018
11ba5f6
update comment
YY-OnCall Dec 17, 2018
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
Prev Previous commit
Next Next commit
remove unnecessary setter
  • Loading branch information
@YY-OnCall
YY-OnCall committed Dec 14, 2018
commit ecef1d0629cf0c1edeb51832247ffe2ea1ee18f4
18 changes: 1 addition & 17 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 @@ -1018,16 +1018,12 @@ class LogisticRegressionModel private[spark] (
@Since("1.6.0")
override val numFeatures: Int = coefficientMatrix.numCols

private var trainingSummary: Option[LogisticRegressionTrainingSummary] = None

/**
* Gets summary of model on training set. An exception is thrown
* if `trainingSummary == None`.
*/
@Since("1.5.0")
def summary: LogisticRegressionTrainingSummary = trainingSummary.getOrElse {
throw new SparkException("No training summary available for this LogisticRegressionModel")
}
override def summary: LogisticRegressionTrainingSummary = super.summary

/**
* Gets summary of model on training set. An exception is thrown
Expand Down Expand Up @@ -1062,18 +1058,6 @@ class LogisticRegressionModel private[spark] (
(model, model.getProbabilityCol, model.getPredictionCol)
}

private[classification] override def setSummary(
summary: Option[LogisticRegressionTrainingSummary]
): this.type = {
super.setSummary(summary)
}

/**
* Gets summary of model on training set. An exception is thrown if `trainingSummary == None`.
*/
@Since("1.5.0")
override def summary: LogisticRegressionTrainingSummary = super.summary

/**
* Evaluates the model on a test dataset.
*
Expand Down
3 changes: 0 additions & 3 deletions mllib/src/main/scala/org/apache/spark/ml/clustering/BisectingKMeans.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ class BisectingKMeansModel private[ml] (
@Since("2.0.0")
override def write: MLWriter = new BisectingKMeansModel.BisectingKMeansModelWriter(this)

private[ml] override def setSummary(summary: Option[BisectingKMeansSummary]): this.type =
super.setSummary(summary)

/**
* Gets summary of model on training set. An exception is
* thrown if `trainingSummary == None`.
Expand Down
3 changes: 0 additions & 3 deletions mllib/src/main/scala/org/apache/spark/ml/clustering/GaussianMixture.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,6 @@ class GaussianMixtureModel private[ml] (
@Since("2.0.0")
override def write: MLWriter = new GaussianMixtureModel.GaussianMixtureModelWriter(this)

private[ml] override def setSummary(summary: Option[GaussianMixtureSummary]): this.type =
super.setSummary(summary)

/**
* Gets summary of model on training set. An exception is
* thrown if `trainingSummary == None`.
Expand Down
3 changes: 0 additions & 3 deletions mllib/src/main/scala/org/apache/spark/ml/clustering/KMeans.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,6 @@ class KMeansModel private[ml] (
@Since("1.6.0")
override def write: GeneralMLWriter = new GeneralMLWriter(this)

private[ml] override def setSummary(summary: Option[KMeansSummary]): this.type =
super.setSummary(summary)

/**
* Gets summary of model on training set. An exception is
* thrown if `trainingSummary == None`.
Expand Down
4 changes: 0 additions & 4 deletions mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1055,10 +1055,6 @@ class GeneralizedLinearRegressionModel private[ml] (
output.toDF()
}

private[ml]
override def setSummary(summary: Option[GeneralizedLinearRegressionTrainingSummary]): this.type =
super.setSummary(summary)

/**
* Gets R-like summary of model on training set. An exception is
* thrown if there is no summary available.
Expand Down
6 changes: 0 additions & 6 deletions 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 @@ -653,14 +653,8 @@ class LinearRegressionModel private[ml] (
private[ml] def this(uid: String, coefficients: Vector, intercept: Double) =
this(uid, coefficients, intercept, 1.0)

private var trainingSummary: Option[LinearRegressionTrainingSummary] = None

override val numFeatures: Int = coefficients.size

private[ml]
override def setSummary(summary: Option[LinearRegressionTrainingSummary]): this.type =
super.setSummary(summary)

/**
* Gets summary (e.g. residuals, mse, r-squared ) of model on training set. An exception is
* thrown if `trainingSummary == None`.
Expand Down
6 changes: 3 additions & 3 deletions mllib/src/main/scala/org/apache/spark/ml/util/HasTrainingSummary.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@ import org.apache.spark.annotation.Since
*
* @tparam T Summary instance type
*/
@Since("2.3.0")
@Since("3.0.0")
private[ml] trait HasTrainingSummary[T] {

private[ml] final var trainingSummary: Option[T] = None

/** Indicates whether a training summary exists for this model instance. */
@Since("2.3.0")
@Since("3.0.0")
def hasSummary: Boolean = trainingSummary.isDefined

/**
* Gets summary of model on training set. An exception is
* thrown if `trainingSummary == None`.
Copy link
Member

@srowen srowen Dec 14, 2018

Choose a reason for hiding this comment

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

Nit: from the callers perspective they don't know what trainingSummary is. "if hasSummary is false"?

Copy link
Member

@srowen srowen Dec 16, 2018

Choose a reason for hiding this comment

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

One more nit @hhbyyh - can we change this one too?

Copy link
Contributor Author

@hhbyyh hhbyyh Dec 17, 2018

Choose a reason for hiding this comment

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

Sure. Thanks for checking.

*/
@Since("2.3.0")
@Since("3.0.0")
def summary: T = trainingSummary.getOrElse {
throw new SparkException(
s"No training summary available for this ${this.getClass.getSimpleName}")
Expand Down