You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sueann <gi...@git.apache.org> on 2017/01/05 21:45:50 UTC

[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

GitHub user sueann opened a pull request:

    https://github.com/apache/spark/pull/16480

    [SPARK-18194][ML] Log instrumentation in OneVsRest, CrossValidator, TrainValidationSplit

    ## What changes were proposed in this pull request?
    
    Added instrumentation logging for OneVsRest classifier, CrossValidator, TrainValidationSplit fit() functions.
    
    ## How was this patch tested?
    
    Ran unit tests and checked the log file. 
    
    * TrainValidationSplit
    ```
    $ build/sbt "test-only org.apache.spark.ml.tuning.TrainValidationSplitSuite\u201d
    $ less mllib/target/unit-tests.log 
    ...
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: training: numPartitions=2 storageLevel=StorageLevel(1 replicas)
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: {"trainRatio":0.5}
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: {"estimator":"LinearRegression","evaluator":"RegressionEvaluator","numModels":4}
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO TrainValidationSplit: Train validation split metrics: WrappedArray(7.79718164555543, 7.734602725775261, 7.79718164555543, 0.11150408019144314)
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO TrainValidationSplit: Best set of parameters:
    {
            linReg_8e7bf3d889f1-maxIter: 10,
            linReg_8e7bf3d889f1-regParam: 0.001
    }
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO TrainValidationSplit: Best train validation split metric: 0.11150408019144314.
    ...
    17/01/05 11:21:52.087 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: training finished
    ```
    
    * CrossValdator
    ```
    $ build/sbt "test-only org.apache.spark.ml.tuning.CrossValidatorSuite"
    $ less mllib/target/unit-tests.log 
    ...
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: training: numPartitions=2 storageLevel=StorageLevel(1 replicas)
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: {"numFolds":3}
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: {"estimator":"LogisticRegression","evaluator":"BinaryClassificationEvaluator","numModels":4}
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO CrossValidator: Average cross-validation metrics: WrappedArray(0.5, 0.5, 0.8333037031406596, 0.8333037031406596)
    17/01/05 11:27:26.862 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO CrossValidator: Best set of parameters:
    {
            logreg_e763c2efb948-maxIter: 10,
            logreg_e763c2efb948-regParam: 0.001
    }
    17/01/05 11:27:26.863 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO CrossValidator: Best cross-validation metric: 0.8333037031406596.
    ...
    17/01/05 11:27:27.040 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: training finished
    ...
    ```
    
    * OneVsRest
    ```
    $ build/sbt "test-only org.apache.spark.ml.classification.OneVsRestSuite"
    $ less mllib/target/unit-tests.log 
    ...
    17/01/05 13:26:36.017 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: training: numPartitions=2 storageLevel=StorageLevel(1 replicas)
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: {"labelCol":"indexed","featuresCol":"f","predictionCol":"p"}
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: {"numClasses":3}
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: {"classifier":"LogisticRegression"}
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: training finished
    ...
    ```


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sueann/spark SPARK-18194

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/16480.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #16480
    
----
commit 84625f2e09b7d35f5487f26c3900c28c5b9d3caa
Author: sueann <su...@databricks.com>
Date:   2017-01-05T06:39:05Z

    [SPARK-18194] log stuff

commit 327c9ff1d809c6bb526916fd20f08e3c5507636e
Author: sueann <su...@databricks.com>
Date:   2017-01-05T21:36:09Z

    [SPARK-18194] instrumentation logging for OneVsRest, CrossValidator, TrainValidationSplit

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    add to whitelist


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94861932
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/util/Instrumentation.scala ---
    @@ -85,9 +86,27 @@ private[spark] class Instrumentation[E <: Estimator[_]] private (
       }
     
       /**
    +   * Log params for tuning estimators that search for the best params by wrapping another
    +   * estimator and using metrics computed by an evaluator.
    +   *
    +   * @param estimator the inner estimator called by the tuning estimator
    +   * @param estimatorParamMaps different params tried by the tuning estimator
    +   * @param evaluator evaluator used to compute the metric for each estimator param value
    +   */
    +  def logTuningParams(
    +    estimator: Estimator[_],
    +    estimatorParamMaps: Array[ParamMap],
    +    evaluator: Evaluator): Unit = {
    +    log(compact(render(map2jvalue(Map[String, JValue](
    +      "estimator" -> estimator.getClass.getSimpleName,
    +      "evaluator" -> evaluator.getClass.getSimpleName,
    +      "numModels" -> estimatorParamMaps.length)))))
    +  }
    +
    +  /**
        * Logs the value with customized name field.
        */
    -  def logNamedValue(name: String, num: Long): Unit = {
    +  def logNamedValue(name: String, num: JValue): Unit = {
    --- End diff --
    
    Since this class doesn't expose json4s APIs, let's stick with basic types in the public API.  Just do String for now (since the Long version is not yet used).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by sueann <gi...@git.apache.org>.
Github user sueann commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Unit test runs & logs:
    
    * TrainValidationSplit
    ```
    $ build/sbt "test-only org.apache.spark.ml.tuning.TrainValidationSplitSuite\u201d
    $ less mllib/target/unit-tests.log 
    ...
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: training: numPartitions=2 storageLevel=StorageLevel(1 replicas)
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: {"trainRatio":0.5}
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: {"estimator":"LinearRegression","evaluator":"RegressionEvaluator","numModels":4}
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO TrainValidationSplit: Train validation split metrics: WrappedArray(7.79718164555543, 7.734602725775261, 7.79718164555543, 0.11150408019144314)
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO TrainValidationSplit: Best set of parameters:
    {
            linReg_8e7bf3d889f1-maxIter: 10,
            linReg_8e7bf3d889f1-regParam: 0.001
    }
    17/01/05 11:21:51.877 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO TrainValidationSplit: Best train validation split metric: 0.11150408019144314.
    ...
    17/01/05 11:21:52.087 pool-1-thread-1-ScalaTest-running-TrainValidationSplitSuite INFO Instrumentation: TrainValidationSplit-tvs_8e7090288e01-206645650-7: training finished
    ```
    
    * CrossValdator
    ```
    $ build/sbt "test-only org.apache.spark.ml.tuning.CrossValidatorSuite"
    $ less mllib/target/unit-tests.log 
    ...
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: training: numPartitions=2 storageLevel=StorageLevel(1 replicas)
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: {"numFolds":3}
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: {"estimator":"LogisticRegression","evaluator":"BinaryClassificationEvaluator","numModels":4}
    17/01/05 11:27:26.860 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO CrossValidator: Average cross-validation metrics: WrappedArray(0.5, 0.5, 0.8333037031406596, 0.8333037031406596)
    17/01/05 11:27:26.862 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO CrossValidator: Best set of parameters:
    {
            logreg_e763c2efb948-maxIter: 10,
            logreg_e763c2efb948-regParam: 0.001
    }
    17/01/05 11:27:26.863 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO CrossValidator: Best cross-validation metric: 0.8333037031406596.
    ...
    17/01/05 11:27:27.040 pool-1-thread-1-ScalaTest-running-CrossValidatorSuite INFO Instrumentation: CrossValidator-cv_f60fd2c0f5ce-428174643-13: training finished
    ...
    ```
    
    * OneVsRest
    ```
    $ build/sbt "test-only org.apache.spark.ml.classification.OneVsRestSuite"
    $ less mllib/target/unit-tests.log 
    ...
    17/01/05 13:26:36.017 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: training: numPartitions=2 storageLevel=StorageLevel(1 replicas)
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: {"labelCol":"indexed","featuresCol":"f","predictionCol":"p"}
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: {"numClasses":3}
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: {"classifier":"LogisticRegression"}
    17/01/05 13:26:36.018 pool-1-thread-1-ScalaTest-running-OneVsRestSuite INFO Instrumentation: OneVsRest-oneVsRest_551a9c8d41e4-1366242214-12: training finished
    ...
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Merged build finished. Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94860559
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
    @@ -344,6 +344,10 @@ final class OneVsRest @Since("1.4.0") (
           multiclassLabeled.unpersist()
         }
     
    +    val instrLog = Instrumentation.create(this, multiclassLabeled)
    --- End diff --
    
    Btw, can you please rename this to "instr" to match other classes?  I see ALS is also named instrLog, but it's the only one.  Could you change ALS to "instr" as well in this PR?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94872466
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
    @@ -339,11 +344,13 @@ final class OneVsRest @Since("1.4.0") (
           paramMap.put(classifier.predictionCol -> getPredictionCol)
           classifier.fit(trainingDataset, paramMap)
         }.toArray[ClassificationModel[_, _]]
    +    instr.logNumFeatures(models.head.numFeatures)
     
         if (handlePersistence) {
           multiclassLabeled.unpersist()
         }
     
    +
    --- End diff --
    
    remove extra newline


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    **[Test build #70996 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70996/testReport)** for PR 16480 at commit [`0034461`](https://github.com/apache/spark/commit/00344616edfcc11d48fee5775186f26c3d49b118).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by sueann <gi...@git.apache.org>.
Github user sueann commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94859428
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
    @@ -344,6 +344,10 @@ final class OneVsRest @Since("1.4.0") (
           multiclassLabeled.unpersist()
         }
     
    +    val instrLog = Instrumentation.create(this, multiclassLabeled)
    --- End diff --
    
    i think `multiclassLabeled` is the dataset we want, but i'm not sure what it's supposed to be used for in instrumentation.scala so i could be wrong 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    ok to test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    **[Test build #70996 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70996/testReport)** for PR 16480 at commit [`0034461`](https://github.com/apache/spark/commit/00344616edfcc11d48fee5775186f26c3d49b118).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94860281
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
    @@ -344,6 +344,10 @@ final class OneVsRest @Since("1.4.0") (
           multiclassLabeled.unpersist()
         }
     
    +    val instrLog = Instrumentation.create(this, multiclassLabeled)
    --- End diff --
    
    I'd actually use the input "dataset" since it has more information (columns), though either should work.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70972/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    LGTM pending Jenkins tests
    Thanks @sueann !


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70996/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94861627
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/tuning/TrainValidationSplit.scala ---
    @@ -116,13 +116,17 @@ class TrainValidationSplit @Since("1.5.0") (@Since("1.5.0") override val uid: St
         }
         validationDataset.unpersist()
     
    +    val instrLog = Instrumentation.create(this, dataset)
    +    instrLog.logParams(trainRatio)
    --- End diff --
    
    log seed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by sueann <gi...@git.apache.org>.
Github user sueann commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Now the logs show the full class path for the estimator/evaluator/classifier:
    ```
    INFO Instrumentation: CrossValidator-cv_acb968c4de59-968285285-1: {"estimator":"org.apache.spark.ml.classification.LogisticRegression","evaluator":"org.apache.spark.ml.evaluation.BinaryClassificationEvaluator","numModels":4}
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94862191
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/util/Instrumentation.scala ---
    @@ -85,9 +86,27 @@ private[spark] class Instrumentation[E <: Estimator[_]] private (
       }
     
       /**
    +   * Log params for tuning estimators that search for the best params by wrapping another
    +   * estimator and using metrics computed by an evaluator.
    +   *
    +   * @param estimator the inner estimator called by the tuning estimator
    +   * @param estimatorParamMaps different params tried by the tuning estimator
    +   * @param evaluator evaluator used to compute the metric for each estimator param value
    +   */
    +  def logTuningParams(
    --- End diff --
    
    I like that this is separated out for all tuning algorithms, but it belongs in ml.tuning.  How about as a static (object) method in ValidatorParams?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/16480


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by sueann <gi...@git.apache.org>.
Github user sueann commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94870250
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/util/Instrumentation.scala ---
    @@ -85,9 +86,27 @@ private[spark] class Instrumentation[E <: Estimator[_]] private (
       }
     
       /**
    +   * Log params for tuning estimators that search for the best params by wrapping another
    +   * estimator and using metrics computed by an evaluator.
    +   *
    +   * @param estimator the inner estimator called by the tuning estimator
    +   * @param estimatorParamMaps different params tried by the tuning estimator
    +   * @param evaluator evaluator used to compute the metric for each estimator param value
    +   */
    +  def logTuningParams(
    --- End diff --
    
    moved into ValidatorParams as a class method -- let me know if you feel strongly about it being static. thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94872199
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/tuning/ValidatorParams.scala ---
    @@ -76,6 +77,18 @@ private[ml] trait ValidatorParams extends HasSeed with Params {
         }
         est.copy(firstEstimatorParamMap).transformSchema(schema)
       }
    +
    +  /**
    +   * Instrumentation logging for tuning params including the inner estimator and evaluator info.
    +   *
    +   * @param instrumentation instrumentation logger
    +   */
    +  protected def logTuningParams(instrumentation: Instrumentation[_]): Unit = {
    +    instrumentation.log(compact(render(map2jvalue(Map[String, JValue](
    --- End diff --
    
    I'd say just use instrumentation.logNamedValue for each of these, rather than handling JSON here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    **[Test build #70972 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70972/testReport)** for PR 16480 at commit [`aab8dd7`](https://github.com/apache/spark/commit/aab8dd724df9895f6b229edbbecd977bdf8d4fdb).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Merging with master.
    Not backporting unless people request it since this memory leak is very minor.
    Thanks @sueann !


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94861991
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/util/Instrumentation.scala ---
    @@ -85,9 +86,27 @@ private[spark] class Instrumentation[E <: Estimator[_]] private (
       }
     
       /**
    +   * Log params for tuning estimators that search for the best params by wrapping another
    +   * estimator and using metrics computed by an evaluator.
    +   *
    +   * @param estimator the inner estimator called by the tuning estimator
    +   * @param estimatorParamMaps different params tried by the tuning estimator
    +   * @param evaluator evaluator used to compute the metric for each estimator param value
    +   */
    +  def logTuningParams(
    +    estimator: Estimator[_],
    +    estimatorParamMaps: Array[ParamMap],
    +    evaluator: Evaluator): Unit = {
    +    log(compact(render(map2jvalue(Map[String, JValue](
    +      "estimator" -> estimator.getClass.getSimpleName,
    +      "evaluator" -> evaluator.getClass.getSimpleName,
    +      "numModels" -> estimatorParamMaps.length)))))
    +  }
    +
    +  /**
        * Logs the value with customized name field.
        */
    -  def logNamedValue(name: String, num: Long): Unit = {
    +  def logNamedValue(name: String, num: JValue): Unit = {
    --- End diff --
    
    "num" -> "value"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94871938
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/tuning/ValidatorParams.scala ---
    @@ -76,6 +77,18 @@ private[ml] trait ValidatorParams extends HasSeed with Params {
         }
         est.copy(firstEstimatorParamMap).transformSchema(schema)
       }
    +
    +  /**
    +   * Instrumentation logging for tuning params including the inner estimator and evaluator info.
    +   *
    +   * @param instrumentation instrumentation logger
    --- End diff --
    
    I'd remove this comment since it doesn't add info.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    **[Test build #3522 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3522/testReport)** for PR 16480 at commit [`aab8dd7`](https://github.com/apache/spark/commit/aab8dd724df9895f6b229edbbecd977bdf8d4fdb).
     * This patch **fails Spark unit tests**.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    **[Test build #70970 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70970/testReport)** for PR 16480 at commit [`aab8dd7`](https://github.com/apache/spark/commit/aab8dd724df9895f6b229edbbecd977bdf8d4fdb).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94861192
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
    @@ -344,6 +344,10 @@ final class OneVsRest @Since("1.4.0") (
           multiclassLabeled.unpersist()
         }
     
    +    val instrLog = Instrumentation.create(this, multiclassLabeled)
    +    instrLog.logParams(labelCol, featuresCol, predictionCol)
    +    instrLog.logNumClasses(numClasses)
    +    instrLog.logNamedValue("classifier", $(classifier).getClass.getSimpleName)
    --- End diff --
    
    Use getCanonicalName instead


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70970/
    Test FAILed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    **[Test build #3522 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3522/testReport)** for PR 16480 at commit [`aab8dd7`](https://github.com/apache/spark/commit/aab8dd724df9895f6b229edbbecd977bdf8d4fdb).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #16480: [SPARK-18194][ML] Log instrumentation in OneVsRest, Cros...

Posted by sueann <gi...@git.apache.org>.
Github user sueann commented on the issue:

    https://github.com/apache/spark/pull/16480
  
    @jkbradley @thunterdb 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #16480: [SPARK-18194][ML] Log instrumentation in OneVsRes...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/16480#discussion_r94860863
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/OneVsRest.scala ---
    @@ -344,6 +344,10 @@ final class OneVsRest @Since("1.4.0") (
           multiclassLabeled.unpersist()
         }
     
    +    val instrLog = Instrumentation.create(this, multiclassLabeled)
    +    instrLog.logParams(labelCol, featuresCol, predictionCol)
    +    instrLog.logNumClasses(numClasses)
    --- End diff --
    
    Also log numFeatures, which you can get from ```models.head.numFeatures```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org