You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by srowen <gi...@git.apache.org> on 2016/06/17 09:06:39 UTC

[GitHub] spark pull request #13729: [SPARK-16008][ML] Remove unnecessary serializatio...

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13729#discussion_r67480171
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala ---
    @@ -937,50 +937,47 @@ class BinaryLogisticRegressionSummary private[classification] (
      * Two LogisticAggregator can be merged together to have a summary of loss and gradient of
      * the corresponding joint dataset.
      *
    - * @param coefficients The coefficients corresponding to the features.
      * @param numClasses the number of possible outcomes for k classes classification problem in
      *                   Multinomial Logistic Regression.
      * @param fitIntercept Whether to fit an intercept term.
    - * @param featuresStd The standard deviation values of the features.
    - * @param featuresMean The mean values of the features.
      */
     private class LogisticAggregator(
    -    coefficients: Vector,
    +    numFeatures: Int,
         numClasses: Int,
    -    fitIntercept: Boolean,
    -    featuresStd: Array[Double],
    -    featuresMean: Array[Double]) extends Serializable {
    +    fitIntercept: Boolean) extends Serializable {
     
       private var weightSum = 0.0
       private var lossSum = 0.0
     
    -  private val coefficientsArray = coefficients match {
    -    case dv: DenseVector => dv.values
    -    case _ =>
    -      throw new IllegalArgumentException(
    -        s"coefficients only supports dense vector but got type ${coefficients.getClass}.")
    -  }
    -
    -  private val dim = if (fitIntercept) coefficientsArray.length - 1 else coefficientsArray.length
    -
    -  private val gradientSumArray = Array.ofDim[Double](coefficientsArray.length)
    +  private val dim = numFeatures
    --- End diff --
    
    Do you need dim here, or can just reference numFeatures later in the class?
    I had to look twice at the line below to make sure the logic wasn't reversed from before but I see why it works out.


---
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