Question

I have a problem with (not anymore with stackoverflow (hehe)) Find algorithm when trying to implement UnionFind structure algorithm with path-compression.

I have standard array of ints, array can get pretty big -> it works fine until 60.000.000 elements.

My Union function looks like this:

public void unite(int p, int q) {
    if(p >= 0 && p < id.length && q >= 0 && q < id.length){
        if (isInSameSet(p, q)) return;
        id[find(p)] = find(q); 
        stevilo--;
    }
}

My isInSameSet looks like this:

    public boolean isInSameSet(int p, int q) {
        if(p >= 0 && p < id.length && q >= 0 && q < id.length) 
            return find(p) == find(q);
        return false;
}

I have tried iterative way in Find:

    public int find(int i) {
        while (i != id[i]){
            id[i] = id[id[i]];
            i = id[i];              
        }       
    return i;
    }

and tail-recrusion:

    public int find(int i) {    
        int p = id[i];
        if (i == p) {
          return i;
        }
        return id[i] = find(p);     
     }

Is there anything I missed in my code? Is there any other approach to this kind of problems?

@edit: Adding constructor to code:

    public UnionFind(int N) {
        stevilo = N;
        id = new int[N];        
        for(int i = 0; i < N; i++){
            id[i] = i;
        }

@edit2 (better explanation and new findings): The problem is not in stackoverflow anymore for less then 60.000.000 elements, which is more then enough for solving my problems.

I'm calling test Unions like this:

for(i=0;i<id.length-1;i++)
unite(i,i+1)

so the ending pairs are like this:

0:1, 1:2, 2:3, 3:4,.. 

which is only example of least optimal option for testing means only :)

Then I check if representative of 0 is last element in table (99 for 100 elements) and it works.

Problem is, that my algorithm works only if initial elements are each in their own union (0:0, 1:1, 2:2, 3:3). If I have different Unions already set up (0:2, 1:6, 2:1, 3:5, ...) my testing algorithm stops working.

I have narrow it down to a problem in Find function, probably something to do with path compression

id[i] = id[id[i]].
Was it helpful?

Solution 3

I once wrote an algorithm for UnionFind, and its time complexity is O(log*(n)). Thats iterative logarithm of n. The algorithm compresses the path of the tree as it keeps on connecting the nodes to gain efficiency. I find it very efficient, though I haven't practically tested it against huge array size. Here's the code:

public class UnionFind
{
  private int[] id;

  public UnionFind(int capacity)
  {
    id = new int[capacity];
    for (int i = 0; i < capacity; i++)
    {
      id[i] = i;
    }
  }

  public boolean isConnected(int p, int q)
  {
    return root(p) == root(q);
  }

  public void connect(int p, int q)
  {
    if (isConnected(p, q))
    {
      return;
    }

    id[root(p)] = root(q);
  }

  private int root(int p)
  {
    int temp = p;

    if (p != id[p] && id[id[p]] != id[p])
    {
      while (p != id[p])
      {
        p = id[p];
      }

      id[temp] = id[p];
    }

    return id[p];
  }
}

OTHER TIPS

One small optimization would be to get rid of isInSameSet...

public void unite(int p, int q) {
    if(p >= 0 && p < id.length && q >= 0 && q < id.length){
        int rootp = find(p);
        int rootq = find(q);
        if (rootp==rootq) return;
        id[rootp] = rootq; 
        stevilo--;
    }
}

Union-Find data structures typically include TWO different optimizations. One is path compression. You have that.

But the other optimization happens during a Union, where you carefully choose which of the two roots to make a child of the other, usually via Union-By-Rank or Union-By-Size. With that optimization, your trees should never be deep enough to get a stack overflow. However, that optimization seems to be missing from your unite function.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top