You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by dwmclary <gi...@git.apache.org> on 2014/03/14 19:02:06 UTC
[GitHub] spark pull request: Spark 1246 add min max to stat counter
GitHub user dwmclary opened a pull request:
https://github.com/apache/spark/pull/144
Spark 1246 add min max to stat counter
Here's the addition of min and max to statscounter.py and min and max methods to rdd.py.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dwmclary/spark SPARK-1246-add-min-max-to-stat-counter
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/144.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 #144
----
commit 491601600a09de1f7f3b46b7459e320d4933cb08
Author: Dan McClary <da...@gmail.com>
Date: 2014-03-11T06:58:32Z
added histogram method, added max and min to statscounter
commit eaf89d957e84d3b926f6c5f3f65acb8764c7ec2f
Author: Dan McClary <da...@gmail.com>
Date: 2014-03-11T07:34:30Z
added correct doctest for histogram
commit 29981f20c28101a34bb15e939062dc24677c4773
Author: Dan McClary <da...@gmail.com>
Date: 2014-03-11T07:57:33Z
fixed indentation on doctest comment
commit 37a7deabb2bfffd7ced1c15d6a079e942b73e7b6
Author: Dan McClary <da...@gmail.com>
Date: 2014-03-11T18:44:22Z
cleaned up boundaries for histogram -- uses real min/max when buckets are derived
commit 1e7056db8a9ce0364e46ad0e269bc1de9f9925f4
Author: Dan McClary <da...@gmail.com>
Date: 2014-03-12T17:42:09Z
added underscore to getBucket
commit ed6713682ac5822b80f23fe1752c058e94033900
Author: Dan McClary <da...@gmail.com>
Date: 2014-03-14T17:56:22Z
broke min/max out into separate transaction, added to rdd.py
----
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10633009
--- Diff: python/pyspark/rdd.py ---
@@ -534,7 +534,26 @@ def func(iterator):
return reduce(op, vals, zeroValue)
# TODO: aggregate
+
+
+ def max(self):
+ """
+ Find the maximum item in this RDD.
+
+ >>> sc.parallelize([1.0, 5.0, 43.0, 10.0]).max()
+ 43.0
+ """
+ return self.stats().max()
--- End diff --
Should use a more direct `reduce` here too
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10633002
--- Diff: core/src/main/scala/org/apache/spark/api/java/JavaRDDLike.scala ---
@@ -477,6 +477,16 @@ trait JavaRDDLike[T, This <: JavaRDDLike[T, This]] extends Serializable {
new java.util.ArrayList(arr)
}
+ def max(comp: Comparator[T]): T = {
--- End diff --
Add doc comments to these and the Scala versions
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37880670
Merged build started.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10643105
--- Diff: core/src/main/scala/org/apache/spark/util/StatCounter.scala ---
@@ -29,6 +29,8 @@ class StatCounter(values: TraversableOnce[Double]) extends Serializable {
private var n: Long = 0 // Running count of our values
private var mu: Double = 0 // Running mean of our values
private var m2: Double = 0 // Running variance numerator (sum of (x - mean)^2)
+ private var max_v: Double = Double.NegativeInfinity // Running max of our values
+ private var min_v: Double = Double.PositiveInfinity // Running min of our values
--- End diff --
Small style nit, both here and in Python -- we don't use underscores in variable names usually, so call these `maxValue` and `minValue`
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10633001
--- Diff: core/src/test/scala/org/apache/spark/PartitioningSuite.scala ---
@@ -171,6 +171,8 @@ class PartitioningSuite extends FunSuite with SharedSparkContext with PrivateMet
assert(abs(6.0/2 - rdd.mean) < 0.01)
assert(abs(1.0 - rdd.variance) < 0.01)
assert(abs(1.0 - rdd.stdev) < 0.01)
+ assert(abs(4.0 - stats.max) === 0)
+ assert(abs(-1.0 - stats.max) === 0)
--- End diff --
Probably easier to write these as stats.max === 4.0 and stats.min === -1.0
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37714253
Yeah sorry, I didn't mean leave out max and min from StatCounter, I just meant that the RDD.max() and RDD.min() methods should directly call reduce. If you're calling those (instead of RDD.stats()), you only want one of the values, so there's no need to compute the others.
@dwmclary, thanks for the update. I made a few small comments but the final thing missing would be to update the Scala StatCounter class with max and min fields as well -- it doesn't currently have them, but you added them to Python. Sorry it's a bit of a pain to maintain consistency across these.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37880669
Merged build triggered.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10633006
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -958,6 +958,10 @@ abstract class RDD[T: ClassTag](
*/
def takeOrdered(num: Int)(implicit ord: Ordering[T]): Array[T] = top(num)(ord.reverse)
+ def max()(implicit ord: Ordering[T]):T = this.reduce{(x,y) => ord.max(x,y)}
--- End diff --
I believe you can do `this.reduce(ord.max)`, and same for the min below
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by dwmclary <gi...@git.apache.org>.
Github user dwmclary commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37718637
@mateiz OK, should be good to go now.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10620079
--- Diff: python/pyspark/rdd.py ---
@@ -24,6 +24,7 @@
import sys
import shlex
import traceback
+from bisect import bisect_right
--- End diff --
Doesn't seem to be 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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37906618
Thanks, merged this.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37777118
Hey, sorry for the delay, I noticed one more small bug. Other than that it looks good.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37878192
Jenkins, retest this please
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/144
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by dwmclary <gi...@git.apache.org>.
Github user dwmclary commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37797230
@mateiz OK -- that should do it.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37884837
Merged build 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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37679298
It might be better to implement `RDD.min` and `RDD.max` with `reduce` directly instead of building a whole StatCounter for them. Also, can you add these to the Java/Scala StatCounter too, and to the RDD there? For those we might want the `min` and `max` methods to take an Ordering in Scala, or a Comparator in Java (look at how `takeOrdered` works).
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37712447
Hey Matei,
For a large dataset someone might wanna do it once, like with stat counter all of the numbers are calculated in one go.
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37681660
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10643101
--- Diff: core/src/main/scala/org/apache/spark/api/java/JavaRDDLike.scala ---
@@ -478,6 +478,28 @@ trait JavaRDDLike[T, This <: JavaRDDLike[T, This]] extends Serializable {
}
/**
+ * Returns the maximum element from this RDD as defined by the specified
+ * Comparator[T].
+ * @params comp the comparator that defines ordering
+ * @return the maximum of the RDD
+ * */
+ def max(comp: Comparator[T]): T = {
+ import scala.collection.JavaConversions._
--- End diff --
I don't think you need to import this if you're going to call Ordering.comparatorToOrdering directly -- was it necessary? It was in some other methods because they used other conversions
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on a diff in the pull request:
https://github.com/apache/spark/pull/144#discussion_r10643096
--- Diff: core/src/main/scala/org/apache/spark/util/StatCounter.scala ---
@@ -121,7 +135,7 @@ class StatCounter(values: TraversableOnce[Double]) extends Serializable {
def sampleStdev: Double = math.sqrt(sampleVariance)
override def toString: String = {
- "(count: %d, mean: %f, stdev: %f)".format(count, mean, stdev)
+ "(count: %d, mean: %f, stdev: %f, max: %f, min: $f)".format(count, mean, stdev, max, min)
--- End diff --
This should be `%f`, not `$f`
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by dwmclary <gi...@git.apache.org>.
Github user dwmclary commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37694293
Matei,
I updated the branch to do just that. Thanks for the review!
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by ScrapCodes <gi...@git.apache.org>.
Github user ScrapCodes commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37712645
Ahh I understood the downside, that would be just for numbers then. makes sense. May be we can have both ?
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by mateiz <gi...@git.apache.org>.
Github user mateiz commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37880322
Jenkins, test this please
---
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.
---
[GitHub] spark pull request: Spark 1246 add min max to stat counter
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/144#issuecomment-37884838
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13217/
---
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.
---