Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -82,7 +82,13 @@ private[shared] object SharedParamsCodeGen {
"all instance weights as 1.0"),
ParamDesc[String]("solver", "the solver algorithm for optimization", finalFields = false),
ParamDesc[Int]("aggregationDepth", "suggested depth for treeAggregate (>= 2)", Some("2"),
isValid = "ParamValidators.gtEq(2)", isExpertParam = true))
isValid = "ParamValidators.gtEq(2)", isExpertParam = true),
ParamDesc[Boolean]("collectSubModels", "If set to false, then only the single best " +
"sub-model will be available after fitting. If set to true, then all sub-models will be " +
"available. Warning: For large models, collecting all sub-models can cause OOMs on the " +
"Spark driver.",
Some("false"), isExpertParam = true)
)

val code = genSharedParams(params)
val file = "src/main/scala/org/apache/spark/ml/param/shared/sharedParams.scala"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,4 +402,21 @@ private[ml] trait HasAggregationDepth extends Params {
/** @group expertGetParam */
final def getAggregationDepth: Int = $(aggregationDepth)
}

/**
* Trait for shared param collectSubModels (default: false).
*/
private[ml] trait HasCollectSubModels extends Params {

/**
* Param for whether to collect a list of sub-models trained during tuning.
* @group expertParam
*/
final val collectSubModels: BooleanParam = new BooleanParam(this, "collectSubModels", "whether to collect a list of sub-models trained during tuning")

setDefault(collectSubModels, false)

/** @group expertGetParam */
final def getCollectSubModels: Boolean = $(collectSubModels)
}
// scalastyle:on
137 changes: 125 additions & 12 deletions mllib/src/main/scala/org/apache/spark/ml/tuning/CrossValidator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

package org.apache.spark.ml.tuning

import java.util.{List => JList}
import java.util.{List => JList, Locale}

import scala.collection.JavaConverters._
import scala.concurrent.Future
Expand All @@ -31,7 +31,7 @@ import org.apache.spark.internal.Logging
import org.apache.spark.ml.{Estimator, Model}
import org.apache.spark.ml.evaluation.Evaluator
import org.apache.spark.ml.param.{IntParam, ParamMap, ParamValidators}
import org.apache.spark.ml.param.shared.HasParallelism
import org.apache.spark.ml.param.shared.{HasCollectSubModels, HasParallelism}
import org.apache.spark.ml.util._
import org.apache.spark.mllib.util.MLUtils
import org.apache.spark.sql.{DataFrame, Dataset}
Expand Down Expand Up @@ -67,7 +67,8 @@ private[ml] trait CrossValidatorParams extends ValidatorParams {
@Since("1.2.0")
class CrossValidator @Since("1.2.0") (@Since("1.4.0") override val uid: String)
extends Estimator[CrossValidatorModel]
with CrossValidatorParams with HasParallelism with MLWritable with Logging {
with CrossValidatorParams with HasParallelism with HasCollectSubModels
with MLWritable with Logging {

@Since("1.2.0")
def this() = this(Identifiable.randomUID("cv"))
Expand Down Expand Up @@ -101,6 +102,21 @@ class CrossValidator @Since("1.2.0") (@Since("1.4.0") override val uid: String)
@Since("2.3.0")
def setParallelism(value: Int): this.type = set(parallelism, value)

/**
* Whether to collect submodels when fitting. If set, we can get submodels from
* the returned model.
*
* Note: If set this param, when you save the returned model, you can set an option
* "persistSubModels" to be "true" before saving, in order to save these submodels.
* You can check documents of
* {@link org.apache.spark.ml.tuning.CrossValidatorModel.CrossValidatorModelWriter}
* for more information.
*
* @group expertSetParam
*/
@Since("2.3.0")
def setCollectSubModels(value: Boolean): this.type = set(collectSubModels, value)

@Since("2.0.0")
override def fit(dataset: Dataset[_]): CrossValidatorModel = {
val schema = dataset.schema
Expand All @@ -117,6 +133,12 @@ class CrossValidator @Since("1.2.0") (@Since("1.4.0") override val uid: String)
instr.logParams(numFolds, seed, parallelism)
logTuningParams(instr)

val collectSubModelsParam = $(collectSubModels)

var subModels: Option[Array[Array[Model[_]]]] = if (collectSubModelsParam) {
Copy link
Contributor

Choose a reason for hiding this comment

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

so this var seems unnecessary, could we just it seems like we'd be better by just collecting modelFutures in copy values (then we can avoid the mutation on L145)

Copy link
Contributor Author

@WeichenXu123 WeichenXu123 Nov 3, 2017

Choose a reason for hiding this comment

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

@holdenk @jkbradley I already thought about this issue. The reason I use this way is:

  1. The modelFutures and foldMetricFutures will be executed in pipelined way, when $(collectSubModels) == false, this will make sure that the model generated in modelFutures will be released in time, so that the maximum memory cost will be numParallelism * sizeof(model). If we use the way of "collecting modelFutures", it will increase the memory cost to be $(estimatorParamMaps).length * sizeof(model) . This is a serious issue which is discussed before.
  2. IMO the mutation on L145 won't influence performance. and it do not need something like lock, there is no race condition.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't follow with #1, if we keep all the models (e.g. set collectSubModelsParam) then the maximum memory cost will be $(estimatorParamMaps).length * sizeof(model) in either case? If we don't keep the models (e.g. set collectSubModelsParam to false) then you don't have to collect the future back at the end and there is no additional overhead.

For #2, It's not that mutation impacts performance, its that it makes the code less easy to reason about for no gain (unless I've misunderstood something about part 1).

Copy link
Contributor Author

@WeichenXu123 WeichenXu123 Nov 4, 2017

Choose a reason for hiding this comment

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

@holdenk Oh, sorry for confusing you. Yes, if set collectSubModelsParam the memory cost will always be $(estimatorParamMaps).length * sizeof(model). According to your suggestion, we have to duplicate code logic (but if i am wrong correct me):

  • When set collectSubModelsParam, we cannot pipeline modelFutures and foldMetricFutures, we should execute modelFutures and collect results first, and modify foldMetricFutures logic, change it into something like following:
val foldMetricFutures = modelResults.zip(epm).map { case (model, paramMap) =>
       Future[Double] {
          val metric = eval.evaluate(model.transform(validationDataset, paramMap))
          logDebug(s"Got metric $metric for model trained with $paramMap.")
          metric
      } (executionContext)
  • When not set collectSubModelsParam, just keep current modelFutures & foldMetricFutures and pipeline them to execute. (Only pipeline them we can save memory cost to numParallelism * sizeof(model).

So, according to your suggestion, it seems need more code. So do you still prefer this way ? Or do you have better way to implement that ?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I didn't follow up on this before. I think that @WeichenXu123 's argument is valid, but please say if there are issues I'm missing @holdenk

Some(Array.fill($(numFolds))(Array.fill[Model[_]](epm.length)(null)))
} else None

// Compute metrics for each model over each split
val splits = MLUtils.kFold(dataset.toDF.rdd, $(numFolds), $(seed))
val metrics = splits.zipWithIndex.map { case ((training, validation), splitIndex) =>
Expand All @@ -125,10 +147,14 @@ class CrossValidator @Since("1.2.0") (@Since("1.4.0") override val uid: String)
logDebug(s"Train split $splitIndex with multiple sets of parameters.")

// Fit models in a Future for training in parallel
val modelFutures = epm.map { paramMap =>
val modelFutures = epm.zipWithIndex.map { case (paramMap, paramIndex) =>
Future[Model[_]] {
val model = est.fit(trainingDataset, paramMap)
model.asInstanceOf[Model[_]]
val model = est.fit(trainingDataset, paramMap).asInstanceOf[Model[_]]

if (collectSubModelsParam) {
subModels.get(splitIndex)(paramIndex) = model
}
model
} (executionContext)
}

Expand Down Expand Up @@ -160,7 +186,8 @@ class CrossValidator @Since("1.2.0") (@Since("1.4.0") override val uid: String)
logInfo(s"Best cross-validation metric: $bestMetric.")
val bestModel = est.fit(dataset, epm(bestIndex)).asInstanceOf[Model[_]]
instr.logSuccess(bestModel)
copyValues(new CrossValidatorModel(uid, bestModel, metrics).setParent(this))
copyValues(new CrossValidatorModel(uid, bestModel, metrics)
.setSubModels(subModels).setParent(this))
}

@Since("1.4.0")
Expand Down Expand Up @@ -244,6 +271,31 @@ class CrossValidatorModel private[ml] (
this(uid, bestModel, avgMetrics.asScala.toArray)
}

private var _subModels: Option[Array[Array[Model[_]]]] = None

private[tuning] def setSubModels(subModels: Option[Array[Array[Model[_]]]])
: CrossValidatorModel = {
_subModels = subModels
this
}

/**
* @return submodels represented in two dimension array. The index of outer array is the
* fold index, and the index of inner array corresponds to the ordering of
* estimatorParamMaps
* @throws IllegalArgumentException if subModels are not available. To retrieve subModels,
* make sure to set collectSubModels to true before fitting.
*/
@Since("2.3.0")
def subModels: Array[Array[Model[_]]] = {
require(_subModels.isDefined, "subModels not available, To retrieve subModels, make sure " +
"to set collectSubModels to true before fitting.")
_subModels.get
}

@Since("2.3.0")
def hasSubModels: Boolean = _subModels.isDefined

@Since("2.0.0")
override def transform(dataset: Dataset[_]): DataFrame = {
transformSchema(dataset.schema, logging = true)
Expand All @@ -260,34 +312,76 @@ class CrossValidatorModel private[ml] (
val copied = new CrossValidatorModel(
uid,
bestModel.copy(extra).asInstanceOf[Model[_]],
avgMetrics.clone())
avgMetrics.clone()
).setSubModels(CrossValidatorModel.copySubModels(_subModels))
copyValues(copied, extra).setParent(parent)
}

@Since("1.6.0")
override def write: MLWriter = new CrossValidatorModel.CrossValidatorModelWriter(this)
override def write: CrossValidatorModel.CrossValidatorModelWriter = {
new CrossValidatorModel.CrossValidatorModelWriter(this)
}
}

@Since("1.6.0")
object CrossValidatorModel extends MLReadable[CrossValidatorModel] {

private[CrossValidatorModel] def copySubModels(subModels: Option[Array[Array[Model[_]]]])
: Option[Array[Array[Model[_]]]] = {
subModels.map(_.map(_.map(_.copy(ParamMap.empty).asInstanceOf[Model[_]])))
}

@Since("1.6.0")
override def read: MLReader[CrossValidatorModel] = new CrossValidatorModelReader

@Since("1.6.0")
override def load(path: String): CrossValidatorModel = super.load(path)

private[CrossValidatorModel]
class CrossValidatorModelWriter(instance: CrossValidatorModel) extends MLWriter {
/**
* Writer for CrossValidatorModel.
* @param instance CrossValidatorModel instance used to construct the writer
*
* CrossValidatorModelWriter supports an option "persistSubModels", with possible values
* "true" or "false". If you set the collectSubModels Param before fitting, then you can
* set "persistSubModels" to "true" in order to persist the subModels. By default,
* "persistSubModels" will be "true" when subModels are available and "false" otherwise.
* If subModels are not available, then setting "persistSubModels" to "true" will cause
* an exception.
*/
@Since("2.3.0")
final class CrossValidatorModelWriter private[tuning] (
instance: CrossValidatorModel) extends MLWriter {

ValidatorParams.validateParams(instance)

override protected def saveImpl(path: String): Unit = {
val persistSubModelsParam = optionMap.getOrElse("persistsubmodels",
if (instance.hasSubModels) "true" else "false")

require(Array("true", "false").contains(persistSubModelsParam.toLowerCase(Locale.ROOT)),
s"persistSubModels option value ${persistSubModelsParam} is invalid, the possible " +
"values are \"true\" or \"false\"")
val persistSubModels = persistSubModelsParam.toBoolean

import org.json4s.JsonDSL._
val extraMetadata = "avgMetrics" -> instance.avgMetrics.toSeq
val extraMetadata = ("avgMetrics" -> instance.avgMetrics.toSeq) ~
("persistSubModels" -> persistSubModels)
ValidatorParams.saveImpl(path, instance, sc, Some(extraMetadata))
val bestModelPath = new Path(path, "bestModel").toString
instance.bestModel.asInstanceOf[MLWritable].save(bestModelPath)
if (persistSubModels) {
require(instance.hasSubModels, "When persisting tuning models, you can only set " +
"persistSubModels to true if the tuning was done with collectSubModels set to true. " +
"To save the sub-models, try rerunning fitting with collectSubModels set to true.")
val subModelsPath = new Path(path, "subModels")
for (splitIndex <- 0 until instance.getNumFolds) {
val splitPath = new Path(subModelsPath, s"fold${splitIndex.toString}")
for (paramIndex <- 0 until instance.getEstimatorParamMaps.length) {
val modelPath = new Path(splitPath, paramIndex.toString).toString
instance.subModels(splitIndex)(paramIndex).asInstanceOf[MLWritable].save(modelPath)
}
}
}
}
}

Expand All @@ -301,11 +395,30 @@ object CrossValidatorModel extends MLReadable[CrossValidatorModel] {

val (metadata, estimator, evaluator, estimatorParamMaps) =
ValidatorParams.loadImpl(path, sc, className)
val numFolds = (metadata.params \ "numFolds").extract[Int]
val bestModelPath = new Path(path, "bestModel").toString
val bestModel = DefaultParamsReader.loadParamsInstance[Model[_]](bestModelPath, sc)
val avgMetrics = (metadata.metadata \ "avgMetrics").extract[Seq[Double]].toArray
val persistSubModels = (metadata.metadata \ "persistSubModels")
.extractOrElse[Boolean](false)

val subModels: Option[Array[Array[Model[_]]]] = if (persistSubModels) {
val subModelsPath = new Path(path, "subModels")
val _subModels = Array.fill(numFolds)(Array.fill[Model[_]](
estimatorParamMaps.length)(null))
for (splitIndex <- 0 until numFolds) {
val splitPath = new Path(subModelsPath, s"fold${splitIndex.toString}")
for (paramIndex <- 0 until estimatorParamMaps.length) {
val modelPath = new Path(splitPath, paramIndex.toString).toString
_subModels(splitIndex)(paramIndex) =
DefaultParamsReader.loadParamsInstance(modelPath, sc)
}
}
Some(_subModels)
} else None

val model = new CrossValidatorModel(metadata.uid, bestModel, avgMetrics)
.setSubModels(subModels)
model.set(model.estimator, estimator)
.set(model.evaluator, evaluator)
.set(model.estimatorParamMaps, estimatorParamMaps)
Expand Down
Loading