From d00d8333986503b94d80cb7377f401ab71614dbc Mon Sep 17 00:00:00 2001 From: Polgreen Date: Wed, 20 Jun 2018 15:36:58 +0200 Subject: [PATCH 1/2] Fix: several grapht functions were uncompilable if ever called edgest is a map from node_indext to some type edget. In order to access the node_indext, you must access the first element of the pair. I've also updated the iterators. --- src/util/graph.h | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/util/graph.h b/src/util/graph.h index a79cf2f4316..f57bd5b9d6c 100644 --- a/src/util/graph.h +++ b/src/util/graph.h @@ -709,12 +709,11 @@ std::size_t grapht::connected_subgraphs( const nodet &node=nodes[n]; - for(typename edgest::const_iterator - it=node.out.begin(); - it!=node.out.end(); - it++) - if(!visited[*it]) - s.push(*it); + for(const auto &o : node.out) + { + if(!visited[o.first]) + s.push(o.first); + } } nr++; @@ -805,20 +804,13 @@ void grapht::make_chordal() const nodet &n=tmp[i]; // connect all the nodes in n.out with each other - - for(typename edgest::const_iterator - it1=n.out.begin(); - it1!=n.out.end(); - it1++) - for(typename edgest::const_iterator - it2=n.out.begin(); - it2!=n.out.end(); - it2++) + for(const auto &o1 : n.out) + for(const auto &o2 : n.out) { - if(*it1!=*it2) + if(o1.first!=o2.first) { - tmp.add_undirected_edge(*it1, *it2); - this->add_undirected_edge(*it1, *it2); + tmp.add_undirected_edge(o1.first, o2.first); + this->add_undirected_edge(o1.first, o2.first); } } From deff59df2254705f4dc39e9ad573e4dc203384bd Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 25 Jun 2018 16:56:10 +0100 Subject: [PATCH 2/2] Add tests for grapht::make_chordal and grapht::connected_subgraphs --- src/util/graph.h | 9 ++ unit/Makefile | 1 + unit/util/graph.cpp | 312 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 322 insertions(+) create mode 100644 unit/util/graph.cpp diff --git a/src/util/graph.h b/src/util/graph.h index f57bd5b9d6c..7189a9ae39d 100644 --- a/src/util/graph.h +++ b/src/util/graph.h @@ -678,6 +678,11 @@ std::vector grapht::depth_limited_search( return src; } +/// Find connected subgraphs in an undirected graph. +/// \param [out] subgraph_nr: will be resized to graph.size() and populated +/// to map node indices onto subgraph numbers. The subgraph numbers are dense, +/// in the range 0 - (number of subgraphs - 1) +/// \return Number of subgraphs found. template std::size_t grapht::connected_subgraphs( std::vector &subgraph_nr) @@ -790,6 +795,10 @@ std::size_t grapht::SCCs(std::vector &subgraph_nr) const return t.scc_count; } +/// Ensure a graph is chordal (contains no 4+-cycles without an edge crossing +/// the cycle) by adding extra edges. Note this adds more edges than are +/// required, including to acyclic graphs or acyclic subgraphs of cyclic graphs, +/// but does at least ensure the graph is not chordal. template void grapht::make_chordal() { diff --git a/unit/Makefile b/unit/Makefile index 318d1bdbb96..ca8920e0c9d 100644 --- a/unit/Makefile +++ b/unit/Makefile @@ -24,6 +24,7 @@ SRC += unit_tests.cpp \ solvers/refinement/string_refinement/sparse_array.cpp \ solvers/refinement/string_refinement/union_find_replace.cpp \ util/expr_cast/expr_cast.cpp \ + util/graph.cpp \ util/irep.cpp \ util/irep_sharing.cpp \ util/message.cpp \ diff --git a/unit/util/graph.cpp b/unit/util/graph.cpp new file mode 100644 index 00000000000..25ac3bf020b --- /dev/null +++ b/unit/util/graph.cpp @@ -0,0 +1,312 @@ +/*******************************************************************\ + +Module: Unit test for graph.h + +Author: Diffblue Ltd + +\*******************************************************************/ + +#include + +#include + +template +static inline bool operator==( + const graph_nodet &gn1, const graph_nodet &gn2) +{ + return gn1.in == gn2.in && gn1.out == gn2.out; +} + +template +static inline bool operator==(const grapht &g1, const grapht &g2) +{ + if(g1.size() != g2.size()) + return false; + + for(typename grapht::node_indext i = 0; i < g1.size(); ++i) + { + if(!(g1[i] == g2[i])) + return false; + } + + return true; +} + +/// To verify make_chordal is working as intended: naively search for a +/// hole (a chordless 4+-cycle) +template +static bool contains_hole( + const grapht &g, + const std::vector::node_indext> &cycle_so_far) +{ + const auto &successors_map = g[cycle_so_far.back()].out; + + // If this node has a triangular edge (one leading to cycle_so_far[-3]) then + // this isn't a hole: + if(cycle_so_far.size() >= 3 && + successors_map.count(cycle_so_far[cycle_so_far.size() - 3]) != 0) + { + return false; + } + + // If this node has an edge leading to any other cycle member (except our + // immediate predecessor) then we've found a hole: + if(cycle_so_far.size() >= 4) + { + for(std::size_t i = 0; i <= cycle_so_far.size() - 4; ++i) + { + if(successors_map.count(cycle_so_far[i]) != 0) + return true; + } + } + + // Otherwise try to extend the cycle: + for(const auto &successor : successors_map) + { + // The input is undirected, so a 2-cycle is always present: + if(cycle_so_far.size() >= 2 && + successor.first == cycle_so_far[cycle_so_far.size() - 2]) + { + continue; + } + + std::vector::node_indext> extended_cycle = cycle_so_far; + extended_cycle.push_back(successor.first); + if(contains_hole(g, extended_cycle)) + return true; + } + + return false; +} + +template +static bool contains_hole(const grapht &g) +{ + // For each node in the graph, check for cycles starting at that node. + // This is pretty dumb, but I figure this formulation is simple enough to + // check by hand and the complexity isn't too bad for small examples. + + for(typename grapht::node_indext i = 0; i < g.size(); ++i) + { + std::vector::node_indext> start_node {i}; + if(contains_hole(g, start_node)) + return true; + } + + return false; +} + +typedef grapht> simple_grapht; + +SCENARIO("graph-make-chordal", + "[core][util][graph]") +{ + GIVEN("An acyclic graph") + { + simple_grapht graph; + simple_grapht::node_indext indices[5]; + + for(int i = 0; i < 5; ++i) + indices[i] = graph.add_node(); + + // Make a graph: 0 <-> 1 <-> 4 + // \-> 2 <-/ + // \-> 3 + graph.add_undirected_edge(indices[0], indices[1]); + graph.add_undirected_edge(indices[0], indices[2]); + graph.add_undirected_edge(indices[0], indices[3]); + graph.add_undirected_edge(indices[1], indices[4]); + graph.add_undirected_edge(indices[2], indices[4]); + + WHEN("The graph is made chordal") + { + simple_grapht chordal_graph = graph; + chordal_graph.make_chordal(); + + THEN("The graph should be unchanged") + { + // This doesn't pass, as make_chordal actually adds triangular edges to + // *all* common neighbours, even nodes that aren't part of a cycle. + // REQUIRE(graph == chordal_graph); + + // At least it shouldn't be chordal! + REQUIRE(!contains_hole(chordal_graph)); + } + } + } + + GIVEN("A cyclic graph that is already chordal") + { + simple_grapht graph; + simple_grapht::node_indext indices[5]; + + for(int i = 0; i < 5; ++i) + indices[i] = graph.add_node(); + + // Make a graph: 0 <-> 1 <-> 2 <-> 0 + // 3 <-> 1 <-> 2 <-> 3 + // 4 <-> 1 <-> 2 <-> 4 + graph.add_undirected_edge(indices[0], indices[1]); + graph.add_undirected_edge(indices[1], indices[2]); + graph.add_undirected_edge(indices[2], indices[0]); + graph.add_undirected_edge(indices[3], indices[1]); + graph.add_undirected_edge(indices[2], indices[3]); + graph.add_undirected_edge(indices[4], indices[1]); + graph.add_undirected_edge(indices[2], indices[4]); + + WHEN("The graph is made chordal") + { + simple_grapht chordal_graph = graph; + chordal_graph.make_chordal(); + + THEN("The graph should be unchanged") + { + // This doesn't pass, as make_chordal actually adds triangular edges to + // *all* common neighbours, even cycles that are already chordal. + // REQUIRE(graph == chordal_graph); + + // At least it shouldn't be chordal! + REQUIRE(!contains_hole(chordal_graph)); + } + } + } + + GIVEN("A simple 4-cycle") + { + simple_grapht graph; + simple_grapht::node_indext indices[4]; + + for(int i = 0; i < 4; ++i) + indices[i] = graph.add_node(); + + graph.add_undirected_edge(indices[0], indices[1]); + graph.add_undirected_edge(indices[1], indices[2]); + graph.add_undirected_edge(indices[2], indices[3]); + graph.add_undirected_edge(indices[3], indices[0]); + + // Check the contains_hole predicate is working as intended: + REQUIRE(contains_hole(graph)); + + WHEN("The graph is made chordal") + { + simple_grapht chordal_graph = graph; + chordal_graph.make_chordal(); + + THEN("The graph should gain a chord") + { + REQUIRE(!contains_hole(chordal_graph)); + } + } + } + + GIVEN("A more complicated graph with a hole") + { + simple_grapht graph; + simple_grapht::node_indext indices[8]; + + for(int i = 0; i < 8; ++i) + indices[i] = graph.add_node(); + + // A 5-cycle: + graph.add_undirected_edge(indices[0], indices[1]); + graph.add_undirected_edge(indices[1], indices[2]); + graph.add_undirected_edge(indices[2], indices[3]); + graph.add_undirected_edge(indices[3], indices[4]); + graph.add_undirected_edge(indices[4], indices[0]); + + // A 3-cycle connected to the 5: + graph.add_undirected_edge(indices[4], indices[5]); + graph.add_undirected_edge(indices[5], indices[6]); + graph.add_undirected_edge(indices[6], indices[4]); + + // Another 3-cycle joined onto the 5: + graph.add_undirected_edge(indices[1], indices[7]); + graph.add_undirected_edge(indices[3], indices[7]); + + // Check we've made the input correctly: + REQUIRE(contains_hole(graph)); + + WHEN("The graph is made chordal") + { + simple_grapht chordal_graph = graph; + chordal_graph.make_chordal(); + + THEN("The graph's 5-cycle should be completed with chords") + { + REQUIRE(!contains_hole(chordal_graph)); + } + } + } +} + +SCENARIO("graph-connected-subgraphs", + "[core][util][graph]") +{ + GIVEN("A connected graph") + { + simple_grapht graph; + simple_grapht::node_indext indices[5]; + + for(int i = 0; i < 5; ++i) + indices[i] = graph.add_node(); + + // Make a graph: 0 <-> 1 <-> 4 + // \-> 2 <-/ + // \-> 3 + graph.add_undirected_edge(indices[0], indices[1]); + graph.add_undirected_edge(indices[0], indices[2]); + graph.add_undirected_edge(indices[0], indices[3]); + graph.add_undirected_edge(indices[1], indices[4]); + graph.add_undirected_edge(indices[2], indices[4]); + + WHEN("We take its connected subgraphs") + { + std::vector subgraphs; + graph.connected_subgraphs(subgraphs); + + REQUIRE(subgraphs.size() == graph.size()); + simple_grapht::node_indext only_subgraph = subgraphs.at(0); + + // Check everything is in one subgraph: + REQUIRE( + subgraphs == + std::vector(graph.size(), only_subgraph)); + } + } + + GIVEN("A graph with three unconnected subgraphs") + { + simple_grapht graph; + simple_grapht::node_indext indices[6]; + + for(int i = 0; i < 6; ++i) + indices[i] = graph.add_node(); + + graph.add_undirected_edge(indices[0], indices[1]); + graph.add_undirected_edge(indices[2], indices[3]); + graph.add_undirected_edge(indices[4], indices[5]); + + WHEN("We take its connected subgraphs") + { + std::vector subgraphs; + graph.connected_subgraphs(subgraphs); + + REQUIRE(subgraphs.size() == graph.size()); + simple_grapht::node_indext first_subgraph = subgraphs.at(0); + simple_grapht::node_indext second_subgraph = subgraphs.at(2); + simple_grapht::node_indext third_subgraph = subgraphs.at(4); + + std::vector expected_subgraphs + { + first_subgraph, + first_subgraph, + second_subgraph, + second_subgraph, + third_subgraph, + third_subgraph + }; + + REQUIRE(subgraphs == expected_subgraphs); + } + } +}