Debosmit Ray
Debosmit Ray

Reputation: 5413

Unknown issue converting python code to java to calculate number of simple connected graphs

The goal of this code: Number of simple connected graphs with N labeled vertices and K unlabeled edges.

Note: This might be considered as a Code Review problem, but after repeated tries, I think that the python and java code have the same functionality. I'm not sure if there is something wrong with the code, or something to do with language intricacies, or my error in overlooking something.

This was for a Google Foobar challenge. I completed it using the above method. I have posted links to all the source code, that tests all possible cases.

The first method works completely. Only issue - it makes O(NK) recursive calls and K is on average quadratic in N. [Full source]

A friend came up with an algorithm to do the same thing with a bottom-up approach. The main functionalities:

def answerHelper(n,k):
    totalGraphs = 0

    for s in range(1,n):
        graphs = 0
        for t in range(0,k+1):
            graphs += answer(s, t) * answer(n - s, k - t)

        graphs = choose(n, s)*s*(n - s) * graphs
        totalGraphs+= graphs

    return totalGraphs/2

F = {}
def answer(n, k):
    if (n, k) in F:
        return F[n, k]

    N = n * (n - 1)/2

    if k is n - 1:
        return int(n ** (n-2))
    if k < n or k > N:
        return 0
    if k == N:
        return 1

    result = ((N - k + 1) * answer(n, k - 1) + answerHelper(n, k - 1)) / k
    F[n, k] = result
    return result

The python fails in 4 cases in comparison with the original working Java code [diffchecker]. I presume this is because of some sort of overflow(?). [Full python source]

I am trying to convert this python code to Java. This is what I have come up with.

static Map<List<Integer>, String> resultMap = new HashMap<List<Integer>, String>();
public static String answer(int N, int K) {
    /* for the case where K > N-1 */
    // check if key is present in the map
    List<Integer> tuple = Arrays.asList(N, K);
    if( resultMap.containsKey(tuple) )
        return resultMap.get(tuple);

    // maximum number of edges in a simply 
    // connected undirected unweighted graph 
    // with n nodes = |N| * |N-1| / 2
    int maxEdges = N * (N-1) / 2;   

    /* for the case where K < N-1 or K > N(N-1)/2 */
    if(K < N-1 || K > maxEdges)
        return BigInteger.ZERO.toString();

    /* for the case where K = N-1 */
    // Cayley's formula applies [https://en.wikipedia.org/wiki/Cayley's_formula].
    // number of trees on n labeled vertices is n^{n-2}.
    if(K == N-1)
        return BigInteger.valueOf((long)Math.pow(N, N-2)).toString();

    /* for the case where K = N(N-1)/2 */
    // if K is the maximum possible 
    // number of edges for the number of 
    // nodes, then there is only one way is 
    // to make a graph (connect each node
    // to all other nodes)
    if(K == maxEdges)
        return BigInteger.ONE.toString();

    // number of edges left from maxEdges if I take away K-1 edges
    BigInteger numWays = BigInteger.valueOf(maxEdges - K + 1);

    // number of graphs possible for each of the numWays edges for a graph that has 1 less edge
    BigInteger numGraphsWithOneLessEdge = new BigInteger( answer(N,  K-1) );

    // number of all possible subgraphs with K-1 edges
    BigInteger subGraphs = answerHelper(N, K-1);

    // numWays*numGraphsWithOneLessEdge + subGraphs
    BigInteger result = subGraphs.add(numWays.multiply(numGraphsWithOneLessEdge));

    // this contains repeats for each of the K edges
    result = result.divide(BigInteger.valueOf(K));

    // add to cache
    resultMap.put(Collections.unmodifiableList(Arrays.asList(N, K)), result.toString());
    return resultMap.get(tuple);
}

private static BigInteger answerHelper(int N, int K) {

    BigInteger totalGraphs = BigInteger.ZERO;

    for(int n = 1 ; n < N ; n++) {
        BigInteger graphs = BigInteger.ZERO;
        for(int k = 0 ; k <= K ; k++) {
            // number of graphs with n nodes and k edges
            BigInteger num = new BigInteger( answer(n, k) );

            // number of graphs with N-n nodes and K-k edges
            BigInteger num2 = new BigInteger( answer(N-n, K-k) );

            graphs = graphs.add( num.multiply(num2) );
        }

        // number of ways to choose n nodes from N nodes
        BigInteger choose = choose(N, n);

        // this is repeated for each of the n chosen nodes
        // and the N-n unchosen nodes
        choose = choose.multiply(BigInteger.valueOf(n)).multiply(BigInteger.valueOf(N-n));

        totalGraphs = totalGraphs.add( choose.multiply(graphs) );

    }

    // now, consider the case where N = 20
    // when n = 2, we compute for N-n = 18
    // when n = 18, we do the same thing again
    // hence, contents of totalGraphs is 2 times
    // of what it should be
    return totalGraphs.divide(BigInteger.valueOf(2));
}

[Full source]

This code, that I intended to function the same as Python, fails multiple cases with respect to the working java code [diffchecker]

I would be very grateful if I can get some guidance.

Upvotes: 0

Views: 79

Answers (1)

Debosmit Ray
Debosmit Ray

Reputation: 5413

The issue was in the Java code, not the Python code (I had suspected an overflow; some meticulous debugging proved otherwise. Its not the easiest comparing numbers with 20 odd digits).

The erroneous code:

/* for the case where K = N-1 */
// Cayley's formula applies [https://en.wikipedia.org/wiki/Cayley's_formula].
// number of trees on n labeled vertices is n^{n-2}.
if(K == N-1)
    return BigInteger.valueOf((long)Math.pow(N, N-2)).toString();

For N>=17, (long)Math.pow(N, N-2) was not accurate. This happened because with greater double values, the gap between consecutive values increases. A double can't represent every integer value within its range, and that's what's going wrong here. It's returning the closest double value to the exact result. Moreover, for double values, the mantissa is 52 bits, which roughly equals 16(?) places of decimals. Hence the overflowiness (not really a word). So, the value being returned was smaller than it should have been. Had to replace this with the following code block.

if(K == N-1) {
    if(N < 2)
        return BigInteger.valueOf((long)Math.pow(N, N-2)).toString();

    // multiply N to itself N-2 times
    BigInteger val = BigInteger.ONE;

    int count = 0;

    while(count++ != N-2)
        val = val.multiply( BigInteger.valueOf( (long)N ) );

    return val.toString();
}

Upvotes: 1

Related Questions