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/02/16 14:35:09 UTC

[GitHub] spark pull request: [SPARK-13339] [DOCS] Clarify commutative / ass...

GitHub user srowen opened a pull request:

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

    [SPARK-13339] [DOCS] Clarify commutative / associative operator requirements for reduce, fold

    Clarify that reduce functions need to be commutative, and fold functions do not
    
    See https://github.com/apache/spark/pull/11091

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

    $ git pull https://github.com/srowen/spark SPARK-13339

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

    https://github.com/apache/spark/pull/11217.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 #11217
    
----
commit 91f61d46cb565f2371e6bf56dd86325ebb195816
Author: Sean Owen <so...@cloudera.com>
Date:   2016-02-16T13:34:16Z

    Clarify that reduce functions need to be commutative, and fold functions do not

----


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#discussion_r53010007
  
    --- Diff: core/src/main/scala/org/apache/spark/api/java/JavaPairRDD.scala ---
    @@ -461,10 +461,10 @@ class JavaPairRDD[K, V](val rdd: RDD[(K, V)])
         fromRDD(rdd.partitionBy(partitioner))
     
       /**
    -   * Merge the values for each key using an associative reduce function. This will also perform
    -   * the merging locally on each mapper before sending results to a reducer, similarly to a
    -   * "combiner" in MapReduce.
    -   */
    +    * Return an RDD containing all pairs of elements with matching keys in `this` and `other`. Each
    --- End diff --
    
    This is not directly related; I just found this doc copy and paste error and fixed it along the way.


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#issuecomment-184754332
  
    **[Test build #51363 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51363/consoleFull)** for PR 11217 at commit [`36a80de`](https://github.com/apache/spark/commit/36a80de75b996e90c9b6c9d9b40470a757ed3d29).
     * 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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#issuecomment-184696896
  
    **[Test build #51363 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51363/consoleFull)** for PR 11217 at commit [`36a80de`](https://github.com/apache/spark/commit/36a80de75b996e90c9b6c9d9b40470a757ed3d29).


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#issuecomment-184754681
  
    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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

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


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#issuecomment-184743100
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51362/
    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: [SPARK-13339] [DOCS] Clarify commutative / ass...

Posted by YPares <gi...@git.apache.org>.
Github user YPares commented on the pull request:

    https://github.com/apache/spark/pull/11217#issuecomment-184687772
  
    @srowen Fold function do not need to be commutative? I thought it was the case only in Python?


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#issuecomment-184742740
  
    **[Test build #51362 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51362/consoleFull)** for PR 11217 at commit [`91f61d4`](https://github.com/apache/spark/commit/91f61d46cb565f2371e6bf56dd86325ebb195816).
     * 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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#issuecomment-184743097
  
    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: [SPARK-13339] [DOCS] Clarify commutative / ass...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:

    https://github.com/apache/spark/pull/11217#issuecomment-186160908
  
    Merged to master


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#discussion_r53012725
  
    --- Diff: R/pkg/R/RDD.R ---
    @@ -632,10 +632,10 @@ setMethod("Filter",
     #' Reduce across elements of an RDD.
     #'
     #' This function reduces the elements of this RDD using the
    -#' specified commutative and associative binary operator.
    +#' specified commutative and associative and commutative binary operator.
    --- End diff --
    
    Ugh, I just realized I added one too many commutatives in a few places.


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:

    https://github.com/apache/spark/pull/11217#issuecomment-184692578
  
    They don't need to be since `fold` will operate on the input in some order. Spark's `fold` is built on `Iterator.fold` which would naturally apply the operator in a fixed order from first to last, so commutativity does not matter. See its doc: http://www.scala-lang.org/api/2.11.7/index.html#scala.collection.Iterator ; most Spark docs for `fold` methods do not mention commutativity either.
    
    The scaladoc does mention the non-trivial things that happen if it's not commutative, but it's not required.


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

    https://github.com/apache/spark/pull/11217#issuecomment-184690410
  
    **[Test build #51362 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51362/consoleFull)** for PR 11217 at commit [`91f61d4`](https://github.com/apache/spark/commit/91f61d46cb565f2371e6bf56dd86325ebb195816).


---
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: [SPARK-13339] [DOCS] Clarify commutative / ass...

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

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