Test and fix minimum equivalent graph: Edge is a reference type!

Author: flowtoolz

Code/Graph+Algorithms/Graph+TransitiveReduction.swift

@@ -11,26 +11,24 @@ public extension Graph
      */
     func makeMinimumEquivalentGraph() -> Graph<NodeID, NodeValue>
     {
-        var indirectReachabilities = Set<Edge>()
+        var nonEssentialEdges = Set<Edge>()
         var consideredAncestorsHash = [Node: Nodes]()
 
         for sourceNode in sources
         {
             // TODO: keep track of visited nodes within each traversal from a source and ignore already visited nodes so we can't get hung up in cycles
 
-            let reachabilities = findIndirectReachabilities(around: sourceNode,
-                                                            reachedAncestors: [],
-                                                            consideredAncestorsHash: &consideredAncestorsHash)
-            
-            indirectReachabilities += reachabilities
+            nonEssentialEdges += findNonEssentialEdges(around: sourceNode,
+                                                       reachedAncestors: [],
+                                                       consideredAncestorsHash: &consideredAncestorsHash)
         }
 
-        return copy(excludedEdges: indirectReachabilities)
+        return copy(excludedEdges: nonEssentialEdges)
     }
 
-    private func findIndirectReachabilities(around node: Node,
-                                            reachedAncestors: Nodes,
-                                            consideredAncestorsHash: inout [Node: Nodes]) -> Set<Edge>
+    private func findNonEssentialEdges(around node: Node,
+                                       reachedAncestors: Nodes,
+                                       consideredAncestorsHash: inout [Node: Nodes]) -> Set<Edge>
     {
         let consideredAncestors = consideredAncestorsHash[node, default: Nodes()]
         let ancestorsToConsider = reachedAncestors - consideredAncestors
@@ -43,7 +41,7 @@ public extension Graph
 
         consideredAncestorsHash[node, default: Set<Node>()] += ancestorsToConsider
 
-        var indirectReachabilities = Set<Edge>()
+        var nonEssentialEdges = Set<Edge>()
 
         // base case: add edges from all reached ancestors to all reachable neighbours of node
 
@@ -53,19 +51,22 @@ public extension Graph
         {
             for ancestor in ancestorsToConsider
             {
-                indirectReachabilities += Edge(from: ancestor, to: descendant)
+                if let nonEssentialEdge = edge(from: ancestor, to: descendant)
+                {
+                    nonEssentialEdges += nonEssentialEdge
+                }
             }
         }
 
         // recursive calls on descendants
 
         for descendant in descendants
         {
-            indirectReachabilities += findIndirectReachabilities(around: descendant,
-                                                                 reachedAncestors: ancestorsToConsider + node,
-                                                                 consideredAncestorsHash: &consideredAncestorsHash)
+            nonEssentialEdges += findNonEssentialEdges(around: descendant,
+                                                       reachedAncestors: ancestorsToConsider + node,
+                                                       consideredAncestorsHash: &consideredAncestorsHash)
         }
 
-        return indirectReachabilities
+        return nonEssentialEdges
     }
 }

Code/Graph+Copying.swift

@@ -17,7 +17,14 @@ extension Graph
     public func copy(includedNodes: OrderedSet<Node>? = nil,
                      includedEdges: Set<Edge>? = nil) -> Graph<NodeID, NodeValue>
     {
-        let actualIncludedNodes = includedNodes ?? OrderedSet(nodesByID.values)
+        let myNodes = OrderedSet(nodesByID.values)
+        
+        if !(includedNodes?.isSubset(of: myNodes) ?? true)
+        {
+            log(warning: "Some nodes to include in the Graph copy are not in the graph.")
+        }
+        
+        let actualIncludedNodes = includedNodes ?? myNodes
         let copiesOfIncludedNodes = actualIncludedNodes.map { Node(id: $0.id, value: $0.value) }
 
         let graphCopy = Graph(nodes: OrderedSet(copiesOfIncludedNodes),

Code/Graph/Graph.swift

@@ -49,7 +49,12 @@ public class Graph<NodeID: Hashable, NodeValue>
                         to targetID: NodeID,
                         count: Int = 1) -> Edge?
     {
-        guard let source = node(for: sourceID), let target = node(for: targetID) else { return nil }
+        guard let source = node(for: sourceID), let target = node(for: targetID) else
+        {
+            log(warning: "Tried to add edge between non-existing node IDs:\nsource ID = \(sourceID)\ntarget ID = \(targetID)")
+            return nil
+        }
+        
         return addEdge(from: source, to: target, count: count)
     }
 

Tests/SwiftNodesTests.swift

@@ -84,5 +84,28 @@ class SwiftNodesTests: XCTestCase {
         XCTAssertEqual(graphCopy.edge(from: "id2", to: "id3")?.count, 2)
     }
 
-    // TODO: Test algorithms
+    func testMinimumEquivalentGraph() {
+        // make original graph
+        let graph = Graph<String, Int> { "id\($0)" }
+        
+        let node1 = graph.insert(1)
+        let node2 = graph.insert(2)
+        let node3 = graph.insert(3)
+        
+        graph.addEdge(from: node1, to: node2)
+        graph.addEdge(from: node2, to: node3)
+        graph.addEdge(from: node1, to: node3)
+        
+        XCTAssertEqual(graph.edges.count, 3)
+        
+        // make MEG
+        let meg = graph.makeMinimumEquivalentGraph()
+        
+        XCTAssertEqual(meg.edges.count, 2)
+        XCTAssertNotNil(meg.edge(from: "id1", to: "id2"))
+        XCTAssertNotNil(meg.edge(from: "id2", to: "id3"))
+        XCTAssertNil(meg.edge(from: "id1", to: "id3"))
+    }
+    
+    // TODO: Test more algorithms
 }