You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yanboliang <gi...@git.apache.org> on 2017/05/19 11:41:48 UTC

[GitHub] spark pull request #18035: [MINOR][SPARKR][ML] Fix coefficients issue and co...

GitHub user yanboliang opened a pull request:

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

    [MINOR][SPARKR][ML] Fix coefficients issue and code cleanup for SparkR linear SVM.

    ## What changes were proposed in this pull request?
    Fix coefficients issue and code cleanup for SparkR linear SVM.
    
    ## How was this patch tested?
    Existing tests.

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

    $ git pull https://github.com/yanboliang/spark svm-r

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

    https://github.com/apache/spark/pull/18035.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 #18035
    
----
commit 1ed3ba0c031b6d45fdaa025f3831020417ce164d
Author: Yanbo Liang <yb...@gmail.com>
Date:   2017-05-19T11:33:27Z

    Code reorg and cleanup for SparkR linear SVM.

----


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77179 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77179/testReport)** for PR 18035 at commit [`207d674`](https://github.com/apache/spark/commit/207d674d750601edc99dad46ed5d9574477529c0).
     * 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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    >  I'd propose to rename spark.svmLinear to spark.svm
    I see your point but `svmLinear` is also a popular name in the caret package. My concern would be coming at too general and end up having to find a strange name later because `svm` is taken, or having parameter name conflicts and so on.
    
    Also from various threads it seems really really unlikely that we will implement non-linear form of svm like you said :)



---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    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 issue #18035: [MINOR][SPARKR][ML] Fix coefficients issue and code clea...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77094 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77094/testReport)** for PR 18035 at commit [`1ed3ba0`](https://github.com/apache/spark/commit/1ed3ba0c031b6d45fdaa025f3831020417ce164d).
     * This patch **fails SparkR unit 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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    @yanboliang Appreciate discussing this matter with me, and it is important to sort this out now. Normally I wouldn't mind either way; but in this case I kinda feel strongly about not making this name change for 2 main reasons:
    
    - first, the work has been done by a contributor. I feel we are at some level undoing his work by making this change now after his work is merged, instead of providing valuable timely feedback during the review process
    - second, being concise is important. I understand the popularly of the search term. Aside from future supportability, naming conflicts etc, I think we choose to name it [LinearSVC](http://people.apache.org/~pwendell/spark-nightly/spark-master-docs/spark-2.3.0-SNAPSHOT-2017_05_22_08_01-cfca011-docs/api/scala/index.html#org.apache.spark.ml.classification.LinearSVC) in Scala because it concisely describes what it does and supports. We could have named it SVM but we didn't? So I'm not sure we should name it `svm` for R. We also didn't call boosted tree `gbm` which is hugely popular, but instead `gbt`. Also, as you are aware, we get a lot of feedback and requests on adding new ML algorithm support in Spark. I think it is very important to set expectation in this case so that people does not search and find `svm` but it doesn't do what people thinks it should do? Unless you think we will go beyond linear and support polynomial etc. at some point? But I think you agree that is rather u
 nlikely.
    
    Anyway, what do you think? 


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with inter...

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

    https://github.com/apache/spark/pull/18035#discussion_r117864689
  
    --- Diff: R/pkg/R/mllib_classification.R ---
    @@ -46,15 +46,16 @@ setClass("MultilayerPerceptronClassificationModel", representation(jobj = "jobj"
     #' @note NaiveBayesModel since 2.0.0
     setClass("NaiveBayesModel", representation(jobj = "jobj"))
     
    -#' linear SVM Model
    +#' Linear SVM Model
     #'
    -#' Fits an linear SVM model against a SparkDataFrame. It is a binary classifier, similar to svm in glmnet package
    +#' Fits a linear SVM model against a SparkDataFrame, similar to svm in e1071 package.
    +#' Currently only supports binary classification model with linear kernal.
    --- End diff --
    
    Do you mean `kernel` instead of `kernal`?


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    let's ignore the appveyor intermitted error - since it passed before simple typo changes


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with inter...

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

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


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77179/testReport)** for PR 18035 at commit [`207d674`](https://github.com/apache/spark/commit/207d674d750601edc99dad46ed5d9574477529c0).


---
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 #18035: [MINOR][SPARKR][ML] Fix coefficients issue and code clea...

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

    https://github.com/apache/spark/pull/18035
  
    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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77181/
    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 #18035: [MINOR][SPARKR][ML] Joint coefficients with inter...

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

    https://github.com/apache/spark/pull/18035#discussion_r117708969
  
    --- Diff: R/pkg/R/mllib_classification.R ---
    @@ -124,39 +125,30 @@ setMethod("predict", signature(object = "LinearSVCModel"),
                 predict_internal(object, newData)
               })
     
    -#  Get the summary of an LinearSVCModel
    +#  Get the summary of a linear SVM model.
     
    -#' @param object an LinearSVCModel fitted by \code{spark.svmLinear}.
    +#' @param object a linear SVM model fitted by \code{spark.svmLinear}.
     #' @return \code{summary} returns summary information of the fitted model, which is a list.
     #'         The list includes \code{coefficients} (coefficients of the fitted model),
    -#'         \code{intercept} (intercept of the fitted model), \code{numClasses} (number of classes),
    -#'         \code{numFeatures} (number of features).
    +#'         \code{numClasses} (number of classes), \code{numFeatures} (number of features).
     #' @rdname spark.svmLinear
     #' @aliases summary,LinearSVCModel-method
     #' @export
     #' @note summary(LinearSVCModel) since 2.2.0
     setMethod("summary", signature(object = "LinearSVCModel"),
               function(object) {
                 jobj <- object@jobj
    -            features <- callJMethod(jobj, "features")
    -            labels <- callJMethod(jobj, "labels")
    -            coefficients <- callJMethod(jobj, "coefficients")
    -            nCol <- length(coefficients) / length(features)
    -            coefficients <- matrix(unlist(coefficients), ncol = nCol)
    -            intercept <- callJMethod(jobj, "intercept")
    +            features <- callJMethod(jobj, "rFeatures")
    +            coefficients <- callJMethod(jobj, "rCoefficients")
    +            coefficients <- as.matrix(unlist(coefficients))
    +            colnames(coefficients) <- c("Estimate")
    +            rownames(coefficients) <- unlist(features)
                 numClasses <- callJMethod(jobj, "numClasses")
                 numFeatures <- callJMethod(jobj, "numFeatures")
    -            if (nCol == 1) {
    --- End diff --
    
    @felixcheung The change here is to make ```coefficients``` matrix has only one column named ```Estimate```. I speculate the original code referred to ```spark.logit``` which supports multiple classification, so it should have multiple columns and each columns' name should be corresponding label. However, ```LinearSVC``` will not support multiple classification in the future, so I simplified it at 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 pull request #18035: [MINOR][SPARKR][ML] Fix coefficients issue and co...

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

    https://github.com/apache/spark/pull/18035#discussion_r117464991
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/r/LinearSVCWrapper.scala ---
    @@ -38,9 +38,17 @@ private[r] class LinearSVCWrapper private (
       private val svcModel: LinearSVCModel =
         pipeline.stages(1).asInstanceOf[LinearSVCModel]
     
    -  lazy val coefficients: Array[Double] = svcModel.coefficients.toArray
    +  lazy val rFeatures: Array[String] = if (svcModel.getFitIntercept) {
    +    Array("(Intercept)") ++ features
    --- End diff --
    
    In R we stack ```intercept``` with other feature names, you can refer spark.glm, spark.logit, spark.survreg.


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    are you targeting these changes for 2.2.0 -  since we are making API/return results changes 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 pull request #18035: [MINOR][SPARKR][ML] Joint coefficients with inter...

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

    https://github.com/apache/spark/pull/18035#discussion_r117545681
  
    --- Diff: R/pkg/R/mllib_classification.R ---
    @@ -111,10 +112,10 @@ setMethod("spark.svmLinear", signature(data = "SparkDataFrame", formula = "formu
                 new("LinearSVCModel", jobj = jobj)
               })
     
    -#  Predicted values based on an LinearSVCModel model
    +#  Predicted values based on a linear SVM model.
    --- End diff --
    
    I think these are intentional - we have `#  Predicted values based on an LogisticRegressionModel model`
    they are prefix by `#` and not in generated doc - only for developers


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with inter...

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

    https://github.com/apache/spark/pull/18035#discussion_r117547378
  
    --- Diff: R/pkg/R/mllib_classification.R ---
    @@ -111,10 +112,10 @@ setMethod("spark.svmLinear", signature(data = "SparkDataFrame", formula = "formu
                 new("LinearSVCModel", jobj = jobj)
               })
     
    -#  Predicted values based on an LinearSVCModel model
    +#  Predicted values based on a linear SVM model.
    --- End diff --
    
    there are a couple of these starting with `#`


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77097 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77097/testReport)** for PR 18035 at commit [`39317c1`](https://github.com/apache/spark/commit/39317c1d06361f50fd80f1bcf6eef97c6123070d).
     * 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 issue #18035: [MINOR][SPARKR][ML] Fix coefficients issue and code clea...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77097 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77097/testReport)** for PR 18035 at commit [`39317c1`](https://github.com/apache/spark/commit/39317c1d06361f50fd80f1bcf6eef97c6123070d).


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    @felixcheung Thanks for your comments. I'm targeting this for 2.2, in case for breaking change. With respect to the name issue, I'm still more prefer to rename to ```spark.svm```. There are lots of R packages which implement same functions, but we should follow the most authoritative or frequently-used packages. For SVM, I think ```e1071``` is the one we should refer, you can check the [search result of ```r svm``` in google](https://www.google.com/webhp?hl=zh-CN&ictx=2&sa=X&ved=0ahUKEwip_7ChnoPUAhWKgFQKHWKECTIQPQgD#newwindow=1&safe=strict&hl=zh-CN&q=r+svm), all items in the first page are ```e1071::svm```. To your concern about potential name conflicts, I think we can prevent it by providing parameters such as classification/regression, kernel function, loss function, etc. However, I'm still open to hear your thoughts.


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77097/
    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 #18035: [MINOR][SPARKR][ML] Joint coefficients with inter...

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

    https://github.com/apache/spark/pull/18035#discussion_r117546252
  
    --- Diff: R/pkg/R/mllib_classification.R ---
    @@ -124,39 +125,30 @@ setMethod("predict", signature(object = "LinearSVCModel"),
                 predict_internal(object, newData)
               })
     
    -#  Get the summary of an LinearSVCModel
    +#  Get the summary of a linear SVM model.
     
    -#' @param object an LinearSVCModel fitted by \code{spark.svmLinear}.
    +#' @param object a linear SVM model fitted by \code{spark.svmLinear}.
     #' @return \code{summary} returns summary information of the fitted model, which is a list.
     #'         The list includes \code{coefficients} (coefficients of the fitted model),
    -#'         \code{intercept} (intercept of the fitted model), \code{numClasses} (number of classes),
    -#'         \code{numFeatures} (number of features).
    +#'         \code{numClasses} (number of classes), \code{numFeatures} (number of features).
     #' @rdname spark.svmLinear
     #' @aliases summary,LinearSVCModel-method
     #' @export
     #' @note summary(LinearSVCModel) since 2.2.0
     setMethod("summary", signature(object = "LinearSVCModel"),
               function(object) {
                 jobj <- object@jobj
    -            features <- callJMethod(jobj, "features")
    -            labels <- callJMethod(jobj, "labels")
    -            coefficients <- callJMethod(jobj, "coefficients")
    -            nCol <- length(coefficients) / length(features)
    -            coefficients <- matrix(unlist(coefficients), ncol = nCol)
    -            intercept <- callJMethod(jobj, "intercept")
    +            features <- callJMethod(jobj, "rFeatures")
    +            coefficients <- callJMethod(jobj, "rCoefficients")
    +            coefficients <- as.matrix(unlist(coefficients))
    +            colnames(coefficients) <- c("Estimate")
    +            rownames(coefficients) <- unlist(features)
                 numClasses <- callJMethod(jobj, "numClasses")
                 numFeatures <- callJMethod(jobj, "numFeatures")
    -            if (nCol == 1) {
    --- End diff --
    
    why not label, intercept? i think they are common in R to include what goes into the model (although in many cases it just include the formula in the model summary)


---
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 #18035: [MINOR][SPARKR][ML] Fix coefficients issue and co...

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

    https://github.com/apache/spark/pull/18035#discussion_r117465188
  
    --- Diff: R/pkg/R/mllib_classification.R ---
    @@ -124,39 +125,30 @@ setMethod("predict", signature(object = "LinearSVCModel"),
                 predict_internal(object, newData)
               })
     
    -#  Get the summary of an LinearSVCModel
    +#  Get the summary of a linear SVM model.
     
    -#' @param object an LinearSVCModel fitted by \code{spark.svmLinear}.
    +#' @param object a linear SVM model fitted by \code{spark.svmLinear}.
     #' @return \code{summary} returns summary information of the fitted model, which is a list.
     #'         The list includes \code{coefficients} (coefficients of the fitted model),
    -#'         \code{intercept} (intercept of the fitted model), \code{numClasses} (number of classes),
    -#'         \code{numFeatures} (number of features).
    +#'         \code{numClasses} (number of classes), \code{numFeatures} (number of features).
     #' @rdname spark.svmLinear
     #' @aliases summary,LinearSVCModel-method
     #' @export
     #' @note summary(LinearSVCModel) since 2.2.0
     setMethod("summary", signature(object = "LinearSVCModel"),
               function(object) {
                 jobj <- object@jobj
    -            features <- callJMethod(jobj, "features")
    -            labels <- callJMethod(jobj, "labels")
    -            coefficients <- callJMethod(jobj, "coefficients")
    -            nCol <- length(coefficients) / length(features)
    -            coefficients <- matrix(unlist(coefficients), ncol = nCol)
    -            intercept <- callJMethod(jobj, "intercept")
    +            features <- callJMethod(jobj, "rFeatures")
    +            coefficients <- callJMethod(jobj, "rCoefficients")
    +            coefficients <- as.matrix(unlist(coefficients))
    +            colnames(coefficients) <- c("Estimate")
    +            rownames(coefficients) <- unlist(features)
                 numClasses <- callJMethod(jobj, "numClasses")
                 numFeatures <- callJMethod(jobj, "numFeatures")
    -            if (nCol == 1) {
    --- End diff --
    
    ML ```LinearSVC``` only supports binary classification, and will not support multiple classification in the future, so we can simplify 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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    @felixcheung I'm OK to keep as it is, thanks for your clarification. What about other changes 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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77181 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77181/testReport)** for PR 18035 at commit [`3c14d15`](https://github.com/apache/spark/commit/3c14d15c0f97f91b0fe69b843fe038b7fc776f2e).
     * 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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    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 issue #18035: [MINOR][SPARKR][ML] Fix coefficients issue and code clea...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77094 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77094/testReport)** for PR 18035 at commit [`1ed3ba0`](https://github.com/apache/spark/commit/1ed3ba0c031b6d45fdaa025f3831020417ce164d).


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77181 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77181/testReport)** for PR 18035 at commit [`3c14d15`](https://github.com/apache/spark/commit/3c14d15c0f97f91b0fe69b843fe038b7fc776f2e).


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77216 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77216/testReport)** for PR 18035 at commit [`5d9afe0`](https://github.com/apache/spark/commit/5d9afe06b665464b06705d618a18a8032255fe1d).


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    **[Test build #77216 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/77216/testReport)** for PR 18035 at commit [`5d9afe0`](https://github.com/apache/spark/commit/5d9afe06b665464b06705d618a18a8032255fe1d).
     * 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 issue #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77216/
    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 issue #18035: [MINOR][SPARKR][ML] Fix coefficients issue and code clea...

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

    https://github.com/apache/spark/pull/18035
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77094/
    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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    Merged into master and branch-2.2. Thanks for reviewing.


---
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 #18035: [MINOR][SPARKR][ML] Joint coefficients with intercept fo...

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

    https://github.com/apache/spark/pull/18035
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/77179/
    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