HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Returning the size of the largest clique in a graph

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thegraphsizecliquelargestreturning

Problem

This function is to return the size of the largest clique in a graph. When I run this code, I am running out of space at runtime. Also, it is insanely slow, so any speed tips would be appreciated.

int cliqueSize(graph &g, std::vector nodes, std::vector edges)
{
    std::vector > previous_cliques;

    // start with all the singular nodes...
    for (unsigned i = 0; i  tmp;
        tmp.push_back(nodes[i]);
        previous_cliques.push_back(tmp);
    }

    for (unsigned clique_size = 1; clique_size  > new_cliques;
        // go through all the nodes to try to add to the clique.
        for (unsigned i = 0; i  potential_clique(previous_cliques[j]);
                    potential_clique.push_back(nodes[i]);

                    // isClique has no issues, just checks if the given graph is a clique...   
                    if (isClique(g, potential_clique, edges))
                    {
                        new_cliques.push_back(potential_clique);
                    }
                }
            }
        }
        // no new cliques? 
        if (new_cliques.size() == 0) { return clique_size; }
        else { previous_cliques = new_cliques; }
    }
    return nodes.size();
}

Solution


  • First of all, start passing things by const reference more. You're passing both nodes and edges by value, which will make a copy.



  • Instead of looping through the containers by index, you should use iterators.



  • Pushing tmp into previous_cliques will make a copy.



The following may be faster:

std::vector tmp(1);
for (...) {
    tmp[0] = nodes[i];
    previous_cliques.push_back(tmp);
}


(If you're worried about scope, just wrap it in braces.)

  • Checking for emptiness with size() may be less efficient than with empty(); unlikely in the case of a vector, though (I think size() must be O(1)).



  • You're not showing us isClique, so it's hard to comment on that. Make sure you're passing be const reference there. Also consider not copying previous_cliques until you know you want to push it into new_cliques and instead handling nodes[i] as a special case (may be faster, may be slower).



  • If you're using C++11, the emplace functions are your friend.

Code Snippets

std::vector<VertexID> tmp(1);
for (...) {
    tmp[0] = nodes[i];
    previous_cliques.push_back(tmp);
}

Context

StackExchange Code Review Q#8893, answer score: 4

Revisions (0)

No revisions yet.