Add edge filters and tests. Process TODO.

Author: flowtoolz

Code/Graph+Algorithms/Graph+Filter.swift

@@ -0,0 +1,32 @@
+public extension Graph
+{
+    func filteredEdges(_ isIncluded: EdgeIDs) -> Graph<NodeID, NodeValue>
+    {
+        var result = self
+        result.filterEdges(isIncluded)
+        return result
+    }
+    
+    func filteredEdges(_ isIncluded: (Edge) throws -> Bool) rethrows -> Graph<NodeID, NodeValue>
+    {
+        var result = self
+        try result.filterEdges(isIncluded)
+        return result
+    }
+    
+    mutating func filterEdges(_ isIncluded: EdgeIDs)
+    {
+        filterEdges { isIncluded.contains($0.id) }
+    }
+    
+    mutating func filterEdges(_ isIncluded: (Edge) throws -> Bool) rethrows
+    {
+        try edges.forEach
+        {
+            if try !isIncluded($0)
+            {
+                removeEdge(with: $0.id)
+            }
+        }
+    }
+}

Code/Graph+Algorithms/Graph+NonEssentialEdges.swift

@@ -3,25 +3,23 @@ import SwiftyToolz
 public extension Graph
 {
     /**
-     Remove edges of the condensation graph that are not in its minimum equivalent graph
+     Remove edges that not in the transitive reduction of the condensation graph
 
      Note that this will not remove any edges that are part of cycles (i.e. part of strongly connected components), as it only considers edges of the condensation graph. This is because it's [algorithmically](https://en.wikipedia.org/wiki/Feedback_arc_set#Hardness) as well as conceptually hard to decide which edges in cycles are "non-essential". We recommend dealing with cycles independently of using this function.
      */
-    mutating func removeNonEssentialEdges()
+    mutating func filterEssentialEdges()
     {
-        removeEdges(with: findNonEssentialEdges())
+        filterEdges(findEssentialEdges())
     }
 
-    // TODO: all this should probably be turned around semantically: "find essential edges" and then the client can still use those to filter the graph. also instead of a mutating `removeNonEssentialEdges`, we probably want something like `makeEssentialSubgraph()`, the client can still overwrite a mutable `Graph` value as in `graph = graph.makeEssentialSubgraph()`
-    
     /**
-     Find edges of the condensation graph that are not in its minimum equivalent graph
+     Find edges that are in the minimum equivalent graph of the condensation graph
 
-     Note that this will not find any edges that are part of cycles (i.e. part of strongly connected components), as it only considers edges of the condensation graph. This is because it's [algorithmically](https://en.wikipedia.org/wiki/Feedback_arc_set#Hardness) as well as conceptually hard to decide which edges in cycles are "non-essential". We recommend dealing with cycles independently of using this function.
+     Note that this includes all edges that are part of cycles (i.e. part of strongly connected components), as it only considers edges of the condensation graph. This is because it's [algorithmically](https://en.wikipedia.org/wiki/Feedback_arc_set#Hardness) as well as conceptually hard to decide which edges in cycles are "non-essential". We recommend dealing with cycles independently of using this function.
      */
-    func findNonEssentialEdges() -> EdgeIDs
+    func findEssentialEdges() -> EdgeIDs
     {
-        var idsOfNonEssentialEdges = EdgeIDs()
+        var idsOfEssentialEdges = EdgeIDs()
 
         // TODO: decomposing the graph into its components is probably legacy from before the algorithm extraction from Codeface ... this also seems to have no performance benefit here ... better just ensure makeCondensationGraph() (or find SCCs) and makeMinimumEquivalentGraph() work on disconnected graphs and then do this algorithm here on the whole graph in one go ...
         // for each component graph individually ...
@@ -59,24 +57,28 @@ public extension Graph
                     continue
                 }
 
-                // skip this edge if it is within the same condensation node (within a strongly connected component)
+                // add this edge if it is within the same condensation node (within a strongly connected component and thereby within a cycle)
 
-                if originCondensationNodeID == destinationCondensationNodeID { continue }
+                if originCondensationNodeID == destinationCondensationNodeID
+                {
+                    idsOfEssentialEdges += edge.id
+                    continue
+                }
 
-                // the edge is essential if its equivalent is in the minimum equivalent condensation graph
+                // the non-cyclic edge is essential if its equivalent is in the minimum equivalent condensation graph
 
                 let condensationEdgeID = CondensationEdge.ID(originCondensationNodeID,
                                                              destinationCondensationNodeID)
 
                 let edgeIsEssential = minimumCondensationGraph.contains(condensationEdgeID)
 
-                if !edgeIsEssential
+                if edgeIsEssential
                 {
-                    idsOfNonEssentialEdges += edge.id
+                    idsOfEssentialEdges += edge.id
                 }
             }
         }
 
-        return idsOfNonEssentialEdges
+        return idsOfEssentialEdges
     }
 }

README.md

@@ -228,7 +228,7 @@ SwiftNodes is already being used in production, but [Codeface](https://codeface.
    1. Make existing algorithms compatible with cyclic graphs (two of them are still not)
    2. General purpose graph traversal algorithms (BFT, DFT, compatible with potentially cyclic graphs)
    3. Model edge weights so they *can* be considered in algorithms like Dijkstra. Do we really need a third type parameter for `Graph`? Or just use `Double` as universal weight type? Do we merge that with edge count or keep both distinct?
-   4. Somewhere around here we should be able to move to version 1.0.0 if documentation is complete and up to date
+   4. Around here we should be able to move to version 1.0.0 if documentation is complete and up to date
    5. Better ways of topological sorting
    6. Approximate the [minimum feedback arc set](https://en.wikipedia.org/wiki/Feedback_arc_set), so Codeface can guess "faulty" or unintended dependencies, i.e. the fewest dependencies that need to be cut in order to break all cycles.
 4. Possibly optimize performance – but only based on measurements and only if measurements show that the optimization yields significant acceleration. Optimizing the algorithms might be more effective than optimizing the data structure itself.

Tests/Algorithms/NonEssentialEdges.swift renamed to Tests/Algorithms/EssentialEdgesTests.swift

@@ -1,30 +1,32 @@
 @testable import SwiftNodes
 import XCTest
 
-class NonEssentialEdgesTests: XCTestCase {
+class EssentialEdgesTests: XCTestCase {
 
     func testEmptyGraph() {
-        XCTAssert(Graph<Int, Int>().findNonEssentialEdges().isEmpty)
+        XCTAssert(Graph<Int, Int>().findEssentialEdges().isEmpty)
     }
 
     func testGraphWithoutEdges() {
         let graph = Graph(values: [1, 2, 3])
 
-        XCTAssert(graph.findNonEssentialEdges().isEmpty)
+        XCTAssert(graph.findEssentialEdges().isEmpty)
     }
 
     func testGraphWithoutTransitiveEdges() {
         let graph = Graph(values: [1, 2, 3],
                           edges: [(1, 2), (2, 3)])
 
-        XCTAssert(graph.findNonEssentialEdges().isEmpty)
+        XCTAssertEqual(graph.findEssentialEdges(),
+                       [.init(1, 2), .init(2, 3)])
     }
 
     func testGraphWithOneTransitiveEdge() {
         let graph = Graph(values: [1, 2, 3],
                           edges: [(1, 2), (2, 3), (1, 3)])
 
-        XCTAssertEqual(graph.findNonEssentialEdges(), [.init(1, 3)])
+        XCTAssertEqual(graph.findEssentialEdges(),
+                       [.init(1, 2), .init(2, 3)])
     }
 
     func testAcyclicGraphWithManyTransitiveEdges() {
@@ -48,30 +50,29 @@ class NonEssentialEdgesTests: XCTestCase {
 
         // only edges between neighbouring numbers are essential (0 -> 1 -> 2 ...)
 
-        var expectedNonEssentialEdges = Set<GraphEdge<Int>.ID>()
+        var expectedEssentialEdges = Set<GraphEdge<Int>.ID>()
 
-        for j in 2 ..< numberOfNodes
+        for i in 1 ..< numberOfNodes
         {
-            for i in 0 ... j - 2
-            {
-                expectedNonEssentialEdges.insert(.init(i, j))
-            }
+            expectedEssentialEdges.insert(.init(i - 1, i))
         }
 
-        XCTAssertEqual(graph.findNonEssentialEdges(), expectedNonEssentialEdges)
+        XCTAssertEqual(graph.findEssentialEdges(), expectedEssentialEdges)
     }
 
-    func testGraphWithTwoCyclesButOnlyEssentialEdges() {
+    func testGraphWithTwoCyclesAndOnlyEssentialEdges() {
         let graph = Graph(values: [1, 2, 3, 4, 5, 6],
                           edges: [(1, 2), (2, 3), (3, 1), (3, 4), (4, 5), (5, 6), (6, 4)])
 
-        XCTAssert(graph.findNonEssentialEdges().isEmpty)
+        XCTAssertEqual(graph.findEssentialEdges(), Set(graph.edgeIDs))
     }
 
     func testGraphWithTwoCyclesAndOneNonEssentialEdge() {
         let graph = Graph(values: [1, 2, 3, 4, 5, 6, 7],
                           edges: [(1, 2), (2, 3), (3, 1), (3, 4), (4, 5), (5, 6), (6, 4), (6, 7), (3, 7)])
 
-        XCTAssertEqual(graph.findNonEssentialEdges(), [.init(3, 7)])
+        let allEdgesExceptNonEssentialOne = Set(graph.edgeIDs).subtracting([.init(3, 7)])
+        
+        XCTAssertEqual(graph.findEssentialEdges(), allEdgesExceptNonEssentialOne)
     }
 }

Tests/Algorithms/FilterTests.swift

@@ -0,0 +1,35 @@
+@testable import SwiftNodes
+import XCTest
+
+class FilterTests: XCTestCase {
+    
+    func testEdgeFilterCopyingGraph() {
+        let graph = Graph(values: [1, 2, 3, 4],
+                          edges: [(1, 2), (2, 3), (3, 4)])
+        
+        let filteredGraph = graph.filteredEdges {
+            $0.originID != 3 && $0.destinationID != 3
+        }
+        
+        let expectedGraph = Graph(values: [1, 2, 3, 4],
+                                  edges: [(1, 2)])
+        
+        // this also compares node neighbour caches, so we also test that the filter correctly updates those...
+        XCTAssertEqual(filteredGraph, expectedGraph)
+    }
+    
+    func testEdgeFilterMutatingGraph() {
+        var graph = Graph(values: [1, 2, 3, 4],
+                          edges: [(1, 2), (2, 3), (3, 4)])
+        
+        graph.filterEdges {
+            $0.originID != 3 && $0.destinationID != 3
+        }
+        
+        let expectedGraph = Graph(values: [1, 2, 3, 4],
+                                  edges: [(1, 2)])
+        
+        // this also compares node neighbour caches, so we also test that the filter correctly updates those...
+        XCTAssertEqual(graph, expectedGraph)
+    }
+}