Uh oh!
There was an error while loading. Please reload this page.
[SPARK-19527][Core] Approximate Size of Intersection of Bloom Filters - #16864
[SPARK-19527][Core] Approximate Size of Intersection of Bloom Filters#16864Bcpoole wants to merge 13 commits into
Conversation
… Also function to create union (non-mutation) of two Bloom filters.
…ance instead of static functions
| package org.apache.spark.util.sketch; | ||
| public class IncompatibleUnionException extends Exception { |
| * Callers must ensure the bloom filters are appropriately sized to avoid saturating them. | ||
| * | ||
| * @throws IncompatibleUnionException if either are null, different classes, or different size or number of hash functions | ||
| */ |
There was a problem hiding this comment.
how about just calling this union?
| /** | ||
| * Swamidass & Baldi (2007) approximation for number of items in a Bloom filter | ||
| */ |
There was a problem hiding this comment.
shouldn't this return a long rather than a double?
There was a problem hiding this comment.
I was debating this due to possible rounding errors.
There was a problem hiding this comment.
yea but that would only be off by 1. I wouldn't worry about that since it is approximate anyway.
There was a problem hiding this comment.
It might be easier to keep it as double because the estimate could be out of bound if the bits are full.
rxin
commented
Feb 9, 2017
cc @mengxr / @tjhunter / @jkbradley is this good to have? |
rxin
commented
Feb 9, 2017
I meant just union, but createUnion ... |
| */ | ||
| public abstract long bitSize(); | ||
| /** |
There was a problem hiding this comment.
Please describe the method first and its properties (approximation error). Then put the reference in @seealso with a permanent link to the paper: https://dx.doi.org/10.1021%2Fci600526a
| /** | ||
| * Swamidass & Baldi (2007) approximation for number of items in a Bloom filter | ||
| */ |
There was a problem hiding this comment.
It might be easier to keep it as double because the estimate could be out of bound if the bits are full.
| */ | ||
| public abstract BloomFilterImpl union(BloomFilter other) throws IncompatibleUnionException; | ||
| /** |
There was a problem hiding this comment.
- Same here. Document the method first and then mention the reference.
How is it different from intersecting two bloom filters and then estimate the number of items? Union might lead to larger approximation error.Okay, I got why. Please also document it.
| @Override | ||
| public double approxItems() { | ||
| double m = bitSize(); |
There was a problem hiding this comment.
Mathis deprecated. Usemath.- Please add a test when
bitsis full. This should returnDouble.Infinity.
There was a problem hiding this comment.
Math is deprecated. Use math.
Assume you were thinking of Scala?
…roxItemsInIntersection. Also added reference to paper
…ntersection of A & B
Bcpoole
left a comment
There was a problem hiding this comment.
Believe requested changes were all handled
jiangxb1987
commented
Jun 19, 2017
ping @mengxr Should we move forward with this PR? |
jiangxb1987
commented
Jun 19, 2017
retest this please |
Test looks good |
WeichenXu123
commented
Sep 2, 2017
@Bcpoole Thanks for this PR. But I want to ask which place in spark can this extension apply to ? e.g. can this algo used in join cost estimating or somewhere else ? But if there is no apparent uses for now, I will decrease priority of reviewing this because there're many PRs accumulated waiting review. |
jiangxb1987
commented
Oct 3, 2017
Should we close this PR since it goes stale? WDYT @WeichenXu123 ? |
WeichenXu123
commented
Nov 6, 2017
@jiangxb1987 yes I agree to close it. |
Closesapache#11494Closesapache#14158Closesapache#16803Closesapache#16864Closesapache#17455Closesapache#17936Closesapache#19377 Added: Closesapache#19380Closesapache#18642Closesapache#18377Closesapache#19632 Added: Closesapache#14471Closesapache#17402Closesapache#17953Closesapache#18607 Also cc srowen vanzin HyukjinKwon gatorsmile cloud-fan to see if you have other PRs to close. Author: Xingbo Jiang <xingbo.jiang@databricks.com> Closesapache#19669 from jiangxb1987/stale-prs.
What changes were proposed in this pull request?
Added functions to get the Swamidass & Baldi (2007) approximation for number of items in a Bloom filter and the intersections of two filters. Added an exception type IncompatibleUnionException mimicing IncompatibleMergeException. As needed for the intersection approximation, there is a function that create the union of two Bloom filters (no mutations).
How was this patch tested?
Manual Tests