[SPARK-5956][MLLIB] Pipeline components should be copyable. #5820

mengxr · 2015-04-30T20:49:38Z

This PR added copy(extra: ParamMap): Params to Params, which makes a copy of the current instance with a randomly generated uid and some extra param values. With this change, we only need to implement fit and transform without extra param values given the default implementation of fit(dataset, extra):

def fit(dataset: DataFrame, extra: ParamMap): Model = {
  copy(extra).fit(dataset)
}

Inside fit and transform, since only the embedded values are used, I added $ as an alias for getOrDefault to make the code easier to read. For example, in LinearRegression.fit we have:

val effectiveRegParam = $(regParam) / yStd
val effectiveL1RegParam = $(elasticNetParam) * effectiveRegParam
val effectiveL2RegParam = (1.0 - $(elasticNetParam)) * effectiveRegParam

Meta-algorithm like Pipeline implements its own copy(extra). So the fitted pipeline model stored all copied stages (no matter whether it is a transformer or a model).

Other changes:

Params$.inheritValues is moved to Params!.copyValues and returns the target instance.
fittingParamMap was removed because the parent carries this information.
validate was renamed to validateParams to be more precise.

TODOs:

add tests for newly added methods
update documentation

@jkbradley @dbtsai

….ml components

SparkQA · 2015-04-30T22:00:02Z

Test build #31451 has finished for PR 5820 at commit b642872.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class Evaluator extends Params
- abstract class PipelineStage extends Params with Logging
- class BinaryClassificationEvaluator extends Evaluator with HasRawPredictionCol with HasLabelCol
This patch does not change any dependencies.

SparkQA · 2015-05-01T08:45:20Z

Test build #31537 has finished for PR 5820 at commit 819dd2d.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class Evaluator extends Params
- abstract class PipelineStage extends Params with Logging
- class BinaryClassificationEvaluator extends Evaluator with HasRawPredictionCol with HasLabelCol

SparkQA · 2015-05-01T10:34:17Z

Test build #31543 has finished for PR 5820 at commit 282a1a8.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class Evaluator extends Params
- abstract class PipelineStage extends Params with Logging
- class BinaryClassificationEvaluator extends Evaluator with HasRawPredictionCol with HasLabelCol

SparkQA · 2015-05-01T21:35:17Z

Test build #31600 has finished for PR 5820 at commit 93e7924.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class Evaluator extends Params
- abstract class PipelineStage extends Params with Logging
- class BinaryClassificationEvaluator extends Evaluator with HasRawPredictionCol with HasLabelCol

SparkQA · 2015-05-01T21:36:35Z

Test build #31594 has finished for PR 5820 at commit 463ecae.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class Evaluator extends Params
- abstract class PipelineStage extends Params with Logging
- class BinaryClassificationEvaluator extends Evaluator with HasRawPredictionCol with HasLabelCol

dbtsai · 2015-05-01T22:37:52Z

I like this approach, and it cleaned up and lowed the maintenance burden a lot.

dbtsai · 2015-05-01T22:38:37Z

mllib/src/main/scala/org/apache/spark/ml/regression/LinearRegression.scala

@@ -24,7 +24,7 @@ import breeze.optimize.{LBFGS => BreezeLBFGS, OWLQN => BreezeOWLQN}
 import breeze.optimize.{CachedDiffFunction, DiffFunction}

 import org.apache.spark.annotation.AlphaComponent
-import org.apache.spark.ml.param.{Params, ParamMap}
+import org.apache.spark.ml.param.{ParamMap, Params}
 import org.apache.spark.ml.param.shared.{HasTol, HasElasticNetParam, HasMaxIter, HasRegParam}


Params is no longer used.

dbtsai · 2015-05-04T01:56:32Z

mllib/src/main/scala/org/apache/spark/ml/Estimator.scala

@@ -34,13 +34,16 @@ abstract class Estimator[M <: Model[M]] extends PipelineStage with Params {
   * Fits a single model to the input data with optional parameters.
   *
   * @param dataset input dataset
-   * @param paramPairs Optional list of param pairs.
+   * @param firstParamPair the first param pair, overwrites embedded params
+   * @param otherParamPairs other param pairs.
   *                   These values override any specified in this Estimator's embedded ParamMap.


indentation

SparkQA · 2015-05-04T08:17:25Z

Test build #31734 has finished for PR 5820 at commit 7bef88d.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class Evaluator extends Params
- abstract class PipelineStage extends Params with Logging
- class BinaryClassificationEvaluator extends Evaluator with HasRawPredictionCol with HasLabelCol

dbtsai · 2015-05-04T17:09:09Z

mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala

-    assert(!schema.fieldNames.contains(predictionColName),
+  protected def validateAndTransformSchema(schema: StructType): StructType = {
+    require(schema($(userCol)).dataType == IntegerType)
+    require(schema($(itemCol)).dataType== IntegerType)


space before ==

space before ==

dbtsai · 2015-05-04T17:32:48Z

LGTM

This PR added `copy(extra: ParamMap): Params` to `Params`, which makes a copy of the current instance with a randomly generated uid and some extra param values. With this change, we only need to implement `fit` and `transform` without extra param values given the default implementation of `fit(dataset, extra)`: ~~~scala def fit(dataset: DataFrame, extra: ParamMap): Model = { copy(extra).fit(dataset) } ~~~ Inside `fit` and `transform`, since only the embedded values are used, I added `$` as an alias for `getOrDefault` to make the code easier to read. For example, in `LinearRegression.fit` we have: ~~~scala val effectiveRegParam = $(regParam) / yStd val effectiveL1RegParam = $(elasticNetParam) * effectiveRegParam val effectiveL2RegParam = (1.0 - $(elasticNetParam)) * effectiveRegParam ~~~ Meta-algorithm like `Pipeline` implements its own `copy(extra)`. So the fitted pipeline model stored all copied stages (no matter whether it is a transformer or a model). Other changes: * `Params$.inheritValues` is moved to `Params!.copyValues` and returns the target instance. * `fittingParamMap` was removed because the `parent` carries this information. * `validate` was renamed to `validateParams` to be more precise. TODOs: * [x] add tests for newly added methods * [ ] update documentation jkbradley dbtsai Author: Xiangrui Meng <meng@databricks.com> Closes #5820 from mengxr/SPARK-5956 and squashes the following commits: 7bef88d [Xiangrui Meng] address comments 05229c3 [Xiangrui Meng] assert -> assertEquals b2927b1 [Xiangrui Meng] organize imports f14456b [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 93e7924 [Xiangrui Meng] add tests for hasParam & copy 463ecae [Xiangrui Meng] merge master 2b954c3 [Xiangrui Meng] update Binarizer 465dd12 [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 282a1a8 [Xiangrui Meng] fix test 819dd2d [Xiangrui Meng] merge master b642872 [Xiangrui Meng] example code runs 5a67779 [Xiangrui Meng] examples compile c76b4d1 [Xiangrui Meng] fix all unit tests 0f4fd64 [Xiangrui Meng] fix some tests 9286a22 [Xiangrui Meng] copyValues to trained models 53e0973 [Xiangrui Meng] move inheritValues to Params and rename it to copyValues 9ee004e [Xiangrui Meng] merge copy and copyWith; rename validate to validateParams d882afc [Xiangrui Meng] test compile f082a31 [Xiangrui Meng] make Params copyable and simply handling of extra params in all spark.ml components (cherry picked from commit e0833c5) Signed-off-by: Xiangrui Meng <meng@databricks.com>

mengxr · 2015-05-04T18:30:15Z

Merged into master and branch-1.4. I will address comments in a follow-up PR. Thanks for reviewing!

jkbradley · 2015-05-04T18:41:57Z

mllib/src/main/scala/org/apache/spark/ml/Pipeline.scala


-  override def validate(paramMap: ParamMap): Unit = {
+  /** @group getParam */
+  def getStages: Array[PipelineStage] = $(stages).clone()


It might be worth putting a comment here about why stages need to be cloned.

jkbradley · 2015-05-04T18:42:25Z

@mengxr Good point about it being unclear which Estimator produced the Model. Not including fittingParamMap sounds fine.

jkbradley · 2015-05-04T18:42:47Z

LGTM too. My comments were all small items.

jkbradley · 2015-05-06T20:07:20Z

A few more to-do items

CrossValidator should make a copy of its underlying Estimator (but not Evaluator, I guess). Same for CrossValidatorModel and its underlying Model. CV also needs to make sure its estimatorParamMaps are mapped from the old estimator to the copy (with a different UID).
- Or should it copy the Evaluator? thinking about shared params
After copying a Params instance, all of the parameters are explicitly set. (I.e., default values are copied to explicitly set values.) That's a little odd.

jkbradley · 2015-05-06T21:08:28Z

mllib/src/main/scala/org/apache/spark/ml/Pipeline.scala

-    // Precedence of ParamMaps: paramMap > this.paramMap > fittingParamMap
-    val map = fittingParamMap ++ extractParamMap(paramMap)
-    stages.foldLeft(schema)((cur, transformer) => transformer.transformSchema(cur, map))
+  override def copy(extra: ParamMap): PipelineModel = {


This should use "extra" and copy its stages

This PR added `copy(extra: ParamMap): Params` to `Params`, which makes a copy of the current instance with a randomly generated uid and some extra param values. With this change, we only need to implement `fit` and `transform` without extra param values given the default implementation of `fit(dataset, extra)`: ~~~scala def fit(dataset: DataFrame, extra: ParamMap): Model = { copy(extra).fit(dataset) } ~~~ Inside `fit` and `transform`, since only the embedded values are used, I added `$` as an alias for `getOrDefault` to make the code easier to read. For example, in `LinearRegression.fit` we have: ~~~scala val effectiveRegParam = $(regParam) / yStd val effectiveL1RegParam = $(elasticNetParam) * effectiveRegParam val effectiveL2RegParam = (1.0 - $(elasticNetParam)) * effectiveRegParam ~~~ Meta-algorithm like `Pipeline` implements its own `copy(extra)`. So the fitted pipeline model stored all copied stages (no matter whether it is a transformer or a model). Other changes: * `Params$.inheritValues` is moved to `Params!.copyValues` and returns the target instance. * `fittingParamMap` was removed because the `parent` carries this information. * `validate` was renamed to `validateParams` to be more precise. TODOs: * [x] add tests for newly added methods * [ ] update documentation jkbradley dbtsai Author: Xiangrui Meng <meng@databricks.com> Closes apache#5820 from mengxr/SPARK-5956 and squashes the following commits: 7bef88d [Xiangrui Meng] address comments 05229c3 [Xiangrui Meng] assert -> assertEquals b2927b1 [Xiangrui Meng] organize imports f14456b [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 93e7924 [Xiangrui Meng] add tests for hasParam & copy 463ecae [Xiangrui Meng] merge master 2b954c3 [Xiangrui Meng] update Binarizer 465dd12 [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 282a1a8 [Xiangrui Meng] fix test 819dd2d [Xiangrui Meng] merge master b642872 [Xiangrui Meng] example code runs 5a67779 [Xiangrui Meng] examples compile c76b4d1 [Xiangrui Meng] fix all unit tests 0f4fd64 [Xiangrui Meng] fix some tests 9286a22 [Xiangrui Meng] copyValues to trained models 53e0973 [Xiangrui Meng] move inheritValues to Params and rename it to copyValues 9ee004e [Xiangrui Meng] merge copy and copyWith; rename validate to validateParams d882afc [Xiangrui Meng] test compile f082a31 [Xiangrui Meng] make Params copyable and simply handling of extra params in all spark.ml components

… Updating ML Doc "Estimator, Transformer, and Param" examples. Updating ML Doc's *"Estimator, Transformer, and Param"* example to use `model.extractParamMap` instead of `model.fittingParamMap`, which no longer exists. mengxr, I believe this addresses (part of) the *update documentation* TODO list item from [PR 5820](#5820). Author: Mike Dusenberry <dusenberrymw@gmail.com> Closes #6514 from dusenberrymw/Fix_ML_Doc_Estimator_Transformer_Param_Example and squashes the following commits: 6366e1f [Mike Dusenberry] Updating instances of model.extractParamMap to model.parent.extractParamMap, since the Params of the parent Estimator could possibly differ from thos of the Model. d850e0e [Mike Dusenberry] Removing all references to "fittingParamMap" throughout Spark, since it has been removed. 0480304 [Mike Dusenberry] Updating the ML Doc "Estimator, Transformer, and Param" Java example to use model.extractParamMap() instead of model.fittingParamMap(), which no longer exists. 7d34939 [Mike Dusenberry] Updating ML Doc "Estimator, Transformer, and Param" example to use model.extractParamMap instead of model.fittingParamMap, which no longer exists. (cherry picked from commit ad06727) Signed-off-by: Joseph K. Bradley <joseph@databricks.com>

… Updating ML Doc "Estimator, Transformer, and Param" examples. Updating ML Doc's *"Estimator, Transformer, and Param"* example to use `model.extractParamMap` instead of `model.fittingParamMap`, which no longer exists. mengxr, I believe this addresses (part of) the *update documentation* TODO list item from [PR 5820](#5820). Author: Mike Dusenberry <dusenberrymw@gmail.com> Closes #6514 from dusenberrymw/Fix_ML_Doc_Estimator_Transformer_Param_Example and squashes the following commits: 6366e1f [Mike Dusenberry] Updating instances of model.extractParamMap to model.parent.extractParamMap, since the Params of the parent Estimator could possibly differ from thos of the Model. d850e0e [Mike Dusenberry] Removing all references to "fittingParamMap" throughout Spark, since it has been removed. 0480304 [Mike Dusenberry] Updating the ML Doc "Estimator, Transformer, and Param" Java example to use model.extractParamMap() instead of model.fittingParamMap(), which no longer exists. 7d34939 [Mike Dusenberry] Updating ML Doc "Estimator, Transformer, and Param" example to use model.extractParamMap instead of model.fittingParamMap, which no longer exists.

This PR added `copy(extra: ParamMap): Params` to `Params`, which makes a copy of the current instance with a randomly generated uid and some extra param values. With this change, we only need to implement `fit` and `transform` without extra param values given the default implementation of `fit(dataset, extra)`: ~~~scala def fit(dataset: DataFrame, extra: ParamMap): Model = { copy(extra).fit(dataset) } ~~~ Inside `fit` and `transform`, since only the embedded values are used, I added `$` as an alias for `getOrDefault` to make the code easier to read. For example, in `LinearRegression.fit` we have: ~~~scala val effectiveRegParam = $(regParam) / yStd val effectiveL1RegParam = $(elasticNetParam) * effectiveRegParam val effectiveL2RegParam = (1.0 - $(elasticNetParam)) * effectiveRegParam ~~~ Meta-algorithm like `Pipeline` implements its own `copy(extra)`. So the fitted pipeline model stored all copied stages (no matter whether it is a transformer or a model). Other changes: * `Params$.inheritValues` is moved to `Params!.copyValues` and returns the target instance. * `fittingParamMap` was removed because the `parent` carries this information. * `validate` was renamed to `validateParams` to be more precise. TODOs: * [x] add tests for newly added methods * [ ] update documentation jkbradley dbtsai Author: Xiangrui Meng <meng@databricks.com> Closes apache#5820 from mengxr/SPARK-5956 and squashes the following commits: 7bef88d [Xiangrui Meng] address comments 05229c3 [Xiangrui Meng] assert -> assertEquals b2927b1 [Xiangrui Meng] organize imports f14456b [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 93e7924 [Xiangrui Meng] add tests for hasParam & copy 463ecae [Xiangrui Meng] merge master 2b954c3 [Xiangrui Meng] update Binarizer 465dd12 [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 282a1a8 [Xiangrui Meng] fix test 819dd2d [Xiangrui Meng] merge master b642872 [Xiangrui Meng] example code runs 5a67779 [Xiangrui Meng] examples compile c76b4d1 [Xiangrui Meng] fix all unit tests 0f4fd64 [Xiangrui Meng] fix some tests 9286a22 [Xiangrui Meng] copyValues to trained models 53e0973 [Xiangrui Meng] move inheritValues to Params and rename it to copyValues 9ee004e [Xiangrui Meng] merge copy and copyWith; rename validate to validateParams d882afc [Xiangrui Meng] test compile f082a31 [Xiangrui Meng] make Params copyable and simply handling of extra params in all spark.ml components

… Updating ML Doc "Estimator, Transformer, and Param" examples. Updating ML Doc's *"Estimator, Transformer, and Param"* example to use `model.extractParamMap` instead of `model.fittingParamMap`, which no longer exists. mengxr, I believe this addresses (part of) the *update documentation* TODO list item from [PR 5820](apache#5820). Author: Mike Dusenberry <dusenberrymw@gmail.com> Closes apache#6514 from dusenberrymw/Fix_ML_Doc_Estimator_Transformer_Param_Example and squashes the following commits: 6366e1f [Mike Dusenberry] Updating instances of model.extractParamMap to model.parent.extractParamMap, since the Params of the parent Estimator could possibly differ from thos of the Model. d850e0e [Mike Dusenberry] Removing all references to "fittingParamMap" throughout Spark, since it has been removed. 0480304 [Mike Dusenberry] Updating the ML Doc "Estimator, Transformer, and Param" Java example to use model.extractParamMap() instead of model.fittingParamMap(), which no longer exists. 7d34939 [Mike Dusenberry] Updating ML Doc "Estimator, Transformer, and Param" example to use model.extractParamMap instead of model.fittingParamMap, which no longer exists.

This PR added `copy(extra: ParamMap): Params` to `Params`, which makes a copy of the current instance with a randomly generated uid and some extra param values. With this change, we only need to implement `fit` and `transform` without extra param values given the default implementation of `fit(dataset, extra)`: ~~~scala def fit(dataset: DataFrame, extra: ParamMap): Model = { copy(extra).fit(dataset) } ~~~ Inside `fit` and `transform`, since only the embedded values are used, I added `$` as an alias for `getOrDefault` to make the code easier to read. For example, in `LinearRegression.fit` we have: ~~~scala val effectiveRegParam = $(regParam) / yStd val effectiveL1RegParam = $(elasticNetParam) * effectiveRegParam val effectiveL2RegParam = (1.0 - $(elasticNetParam)) * effectiveRegParam ~~~ Meta-algorithm like `Pipeline` implements its own `copy(extra)`. So the fitted pipeline model stored all copied stages (no matter whether it is a transformer or a model). Other changes: * `Params$.inheritValues` is moved to `Params!.copyValues` and returns the target instance. * `fittingParamMap` was removed because the `parent` carries this information. * `validate` was renamed to `validateParams` to be more precise. TODOs: * [x] add tests for newly added methods * [ ] update documentation jkbradley dbtsai Author: Xiangrui Meng <meng@databricks.com> Closes apache#5820 from mengxr/SPARK-5956 and squashes the following commits: 7bef88d [Xiangrui Meng] address comments 05229c3 [Xiangrui Meng] assert -> assertEquals b2927b1 [Xiangrui Meng] organize imports f14456b [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 93e7924 [Xiangrui Meng] add tests for hasParam & copy 463ecae [Xiangrui Meng] merge master 2b954c3 [Xiangrui Meng] update Binarizer 465dd12 [Xiangrui Meng] Merge remote-tracking branch 'apache/master' into SPARK-5956 282a1a8 [Xiangrui Meng] fix test 819dd2d [Xiangrui Meng] merge master b642872 [Xiangrui Meng] example code runs 5a67779 [Xiangrui Meng] examples compile c76b4d1 [Xiangrui Meng] fix all unit tests 0f4fd64 [Xiangrui Meng] fix some tests 9286a22 [Xiangrui Meng] copyValues to trained models 53e0973 [Xiangrui Meng] move inheritValues to Params and rename it to copyValues 9ee004e [Xiangrui Meng] merge copy and copyWith; rename validate to validateParams d882afc [Xiangrui Meng] test compile f082a31 [Xiangrui Meng] make Params copyable and simply handling of extra params in all spark.ml components

… Updating ML Doc "Estimator, Transformer, and Param" examples. Updating ML Doc's *"Estimator, Transformer, and Param"* example to use `model.extractParamMap` instead of `model.fittingParamMap`, which no longer exists. mengxr, I believe this addresses (part of) the *update documentation* TODO list item from [PR 5820](apache#5820). Author: Mike Dusenberry <dusenberrymw@gmail.com> Closes apache#6514 from dusenberrymw/Fix_ML_Doc_Estimator_Transformer_Param_Example and squashes the following commits: 6366e1f [Mike Dusenberry] Updating instances of model.extractParamMap to model.parent.extractParamMap, since the Params of the parent Estimator could possibly differ from thos of the Model. d850e0e [Mike Dusenberry] Removing all references to "fittingParamMap" throughout Spark, since it has been removed. 0480304 [Mike Dusenberry] Updating the ML Doc "Estimator, Transformer, and Param" Java example to use model.extractParamMap() instead of model.fittingParamMap(), which no longer exists. 7d34939 [Mike Dusenberry] Updating ML Doc "Estimator, Transformer, and Param" example to use model.extractParamMap instead of model.fittingParamMap, which no longer exists.

mengxr added 9 commits April 30, 2015 01:30

make Params copyable and simply handling of extra params in all spark…

f082a31

….ml components

test compile

d882afc

merge copy and copyWith; rename validate to validateParams

9ee004e

move inheritValues to Params and rename it to copyValues

53e0973

copyValues to trained models

9286a22

fix some tests

0f4fd64

fix all unit tests

c76b4d1

examples compile

5a67779

example code runs

b642872

merge master

819dd2d

fix test

282a1a8

mengxr added 4 commits May 1, 2015 10:02

Merge remote-tracking branch 'apache/master' into SPARK-5956

465dd12

update Binarizer

2b954c3

merge master

463ecae

add tests for hasParam & copy

93e7924

dbtsai reviewed May 1, 2015
View reviewed changes

dbtsai reviewed May 4, 2015
View reviewed changes

address comments

7bef88d

dbtsai reviewed May 4, 2015
View reviewed changes

asfgit closed this in e0833c5 May 4, 2015

jkbradley reviewed May 4, 2015
View reviewed changes

jkbradley reviewed May 6, 2015
View reviewed changes

dusenberrymw mentioned this pull request May 30, 2015

[SPARK-7985] [ML] [MLlib] [Docs] Remove "fittingParamMap" references. Updating ML Doc "Estimator, Transformer, and Param" examples. #6514

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-5956][MLLIB] Pipeline components should be copyable. #5820

[SPARK-5956][MLLIB] Pipeline components should be copyable. #5820

mengxr commented Apr 30, 2015

SparkQA commented Apr 30, 2015

SparkQA commented May 1, 2015

SparkQA commented May 1, 2015

SparkQA commented May 1, 2015

SparkQA commented May 1, 2015

dbtsai commented May 1, 2015

dbtsai May 1, 2015

dbtsai May 4, 2015

mengxr May 4, 2015

SparkQA commented May 4, 2015

dbtsai May 4, 2015

jkbradley May 4, 2015

dbtsai commented May 4, 2015

mengxr commented May 4, 2015

jkbradley May 4, 2015

jkbradley commented May 4, 2015

jkbradley commented May 4, 2015

jkbradley commented May 6, 2015

jkbradley May 6, 2015

[SPARK-5956][MLLIB] Pipeline components should be copyable. #5820

[SPARK-5956][MLLIB] Pipeline components should be copyable. #5820

Conversation

mengxr commented Apr 30, 2015

SparkQA commented Apr 30, 2015

SparkQA commented May 1, 2015

SparkQA commented May 1, 2015

SparkQA commented May 1, 2015

SparkQA commented May 1, 2015

dbtsai commented May 1, 2015

dbtsai May 1, 2015

Choose a reason for hiding this comment

dbtsai May 4, 2015

Choose a reason for hiding this comment

mengxr May 4, 2015

Choose a reason for hiding this comment

SparkQA commented May 4, 2015

dbtsai May 4, 2015

Choose a reason for hiding this comment

jkbradley May 4, 2015

Choose a reason for hiding this comment

dbtsai commented May 4, 2015

mengxr commented May 4, 2015

jkbradley May 4, 2015

Choose a reason for hiding this comment

jkbradley commented May 4, 2015

jkbradley commented May 4, 2015

jkbradley commented May 6, 2015

jkbradley May 6, 2015

Choose a reason for hiding this comment