From 11dceac7f4cbe1f32e32a4fe5f6fed02da65da01 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Fri, 30 May 2025 14:20:35 -0500 Subject: [PATCH 1/2] WIP --- dsm/src/main/java/org/hjug/dsm/DSM.java | 277 ++++++++++++++---- .../java/org/hjug/dsm/EdgeToRemoveInfo.java | 4 +- .../org/hjug/dsm/OptimalBackEdgeRemover.java | 105 +++++++ .../SparseGraphCircularReferenceChecker.java | 71 +++++ .../SparseIntDWGEdgeRemovalCalculator.java | 181 ++++++++++++ dsm/src/test/java/org/hjug/dsm/DSMTest.java | 1 - .../hjug/dsm/OptimalBackEdgeRemoverTest.java | 102 +++++++ .../report/SimpleHtmlReport.java | 35 +-- 8 files changed, 689 insertions(+), 87 deletions(-) create mode 100644 dsm/src/main/java/org/hjug/dsm/OptimalBackEdgeRemover.java create mode 100644 dsm/src/main/java/org/hjug/dsm/SparseGraphCircularReferenceChecker.java create mode 100644 dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java create mode 100644 dsm/src/test/java/org/hjug/dsm/OptimalBackEdgeRemoverTest.java diff --git a/dsm/src/main/java/org/hjug/dsm/DSM.java b/dsm/src/main/java/org/hjug/dsm/DSM.java index 618ecae0..0dedceec 100644 --- a/dsm/src/main/java/org/hjug/dsm/DSM.java +++ b/dsm/src/main/java/org/hjug/dsm/DSM.java @@ -2,6 +2,7 @@ import java.util.*; import java.util.stream.Collectors; + import lombok.Getter; import org.jgrapht.Graph; import org.jgrapht.Graphs; @@ -34,9 +35,16 @@ */ public class DSM { - private Graph graph; + private final Graph graph; private List sortedActivities; boolean activitiesSorted = false; + private final List edgesAboveDiagonal = new ArrayList<>(); + + List sparseIntSortedActivities; + SparseIntDirectedWeightedGraph sparseGraph; + + @Getter + double sumOfEdgeWeightsAboveDiagonal; Map vertexToInt = new HashMap<>(); Map intToVertex = new HashMap<>(); @@ -68,13 +76,14 @@ public void addDependency(String from, String to, int weight) { } private void orderVertices() { - SparseIntDirectedWeightedGraph sparseGraph = getSparseIntDirectedWeightedGraph(); - List> sccs = findStronglyConnectedComponents(sparseGraph); - sortedActivities = convertIntToStringVertices(topologicalSort(sccs, sparseGraph)); + sparseGraph = getSparseIntDirectedWeightedGraph(); + List> sccs = this.findStronglyConnectedSparseGraphComponents(sparseGraph); + sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); // reversing corrects rendering of the DSM // with sources as rows and targets as columns // was needed after AI solution was generated and iterated - Collections.reverse(sortedActivities); + Collections.reverse(sparseIntSortedActivities); + sortedActivities = convertIntToStringVertices(sparseIntSortedActivities); activitiesSorted = true; } @@ -104,23 +113,24 @@ List convertIntToStringVertices(List intVertices) { /** * Kosaraju SCC detector avoids stack overflow. * It is used by JGraphT's CycleDetector, and makes sense to use it here as well for consistency + * * @param graph * @return */ - private List> findStronglyConnectedComponents(Graph graph) { + private List> findStronglyConnectedSparseGraphComponents(Graph graph) { KosarajuStrongConnectivityInspector kosaraju = new KosarajuStrongConnectivityInspector<>(graph); return kosaraju.stronglyConnectedSets(); } - private List topologicalSort(List> sccs, Graph graph) { + private List topologicalSortSparseGraph(List> sccs, Graph graph) { List sortedActivities = new ArrayList<>(); Set visited = new HashSet<>(); for (Set scc : sccs) { for (Integer activity : scc) { if (!visited.contains(activity)) { - topologicalSortUtil(activity, visited, sortedActivities, graph); + topologicalSortUtilSparseGraph(activity, visited, sortedActivities, graph); } } } @@ -129,13 +139,13 @@ private List topologicalSort(List> sccs, Graph visited, List sortedActivities, Graph graph) { visited.add(activity); for (Integer neighbor : Graphs.successorListOf(graph, activity)) { if (!visited.contains(neighbor)) { - topologicalSortUtil(neighbor, visited, sortedActivities, graph); + topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph); } } @@ -147,20 +157,45 @@ public List getEdgesAboveDiagonal() { orderVertices(); } - List edgesAboveDiagonal = new ArrayList<>(); + if (edgesAboveDiagonal.isEmpty()) { + for (int i = 0; i < sortedActivities.size(); i++) { + for (int j = i + 1; j < sortedActivities.size(); j++) { + // source / destination vertex was flipped after solution generation + // to correctly identify the vertex above the diagonal to remove + DefaultWeightedEdge edge = graph.getEdge(sortedActivities.get(i), sortedActivities.get(j)); + if (edge != null) { + edgesAboveDiagonal.add(edge); + } + } + } - for (int i = 0; i < sortedActivities.size(); i++) { - for (int j = i + 1; j < sortedActivities.size(); j++) { + sumOfEdgeWeightsAboveDiagonal = edgesAboveDiagonal.stream() + .mapToInt(edge -> (int) graph.getEdgeWeight(edge)).sum(); + } + + return edgesAboveDiagonal; + } + + private List getSparseEdgesAboveDiagonal() { + if (!activitiesSorted) { + orderVertices(); + } + + List sparseEdgesAboveDiagonal = new ArrayList<>(); + + for (int i = 0; i < sparseIntSortedActivities.size(); i++) { + for (int j = i + 1; j < sparseIntSortedActivities.size(); j++) { // source / destination vertex was flipped after solution generation // to correctly identify the vertex above the diagonal to remove - DefaultWeightedEdge edge = graph.getEdge(sortedActivities.get(i), sortedActivities.get(j)); + Integer edge = sparseGraph.getEdge(sparseIntSortedActivities.get(i), sparseIntSortedActivities.get(j)); + if (edge != null) { - edgesAboveDiagonal.add(edge); + sparseEdgesAboveDiagonal.add(edge); } } } - return edgesAboveDiagonal; + return sparseEdgesAboveDiagonal; } public DefaultWeightedEdge getFirstLowestWeightEdgeAboveDiagonalToRemove() { @@ -243,32 +278,58 @@ void printDSM(Graph graph, List sortedActiv } } + ///////////////////////////////////////////////////////// + // "Standard" Graph implementation to find edge to remove + ///////////////////////////////////////////////////////// + /** * Captures the impact of the removal of each edge above the diagonal. */ - public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) { - // get edges above diagonal for DSM graph - List edgesAboveDiagonal; - List allEdgesAboveDiagonal = getEdgesAboveDiagonal(); - - if (limit == 0 || allEdgesAboveDiagonal.size() <= limit) { - edgesAboveDiagonal = allEdgesAboveDiagonal; - } else { - // get first 50 values of min weight - List minimumWeightEdgesAboveDiagonal = getMinimumWeightEdgesAboveDiagonal(); - int max = Math.min(minimumWeightEdgesAboveDiagonal.size(), limit); - edgesAboveDiagonal = minimumWeightEdgesAboveDiagonal.subList(0, max); - } - - double avgCycleNodeCount = getAverageCycleNodeCount(); + public List getImpactOfEdgesAboveDiagonalIfRemoved() { + +// // get edges above diagonal for DSM graph +// List edgesAboveDiagonal; +// List allEdgesAboveDiagonal = getEdgesAboveDiagonal(); +// +// if (limit == 0 || allEdgesAboveDiagonal.size() <= limit) { +// edgesAboveDiagonal = allEdgesAboveDiagonal; +// } else { +// // get first 50 values of min weight +// List minimumWeightEdgesAboveDiagonal = getMinimumWeightEdgesAboveDiagonal(); +// int max = Math.min(minimumWeightEdgesAboveDiagonal.size(), limit); +// edgesAboveDiagonal = minimumWeightEdgesAboveDiagonal.subList(0, max); +// } + + int currentCycleCount = new CircularReferenceChecker().getCycles(graph).size(); + + return getEdgesAboveDiagonal().stream() + .map(this::calculateEdgeToRemoveInfo) + .sorted(Comparator + .comparing((EdgeToRemoveInfo edgeToRemoveInfo) -> currentCycleCount - edgeToRemoveInfo.getNewCycleCount()) + /*.thenComparing(EdgeToRemoveInfo::getEdgeWeight)*/) + .collect(Collectors.toList()); + } - // build the cloned graph - Graph clonedGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); - graph.vertexSet().forEach(clonedGraph::addVertex); + private EdgeToRemoveInfo calculateEdgeToRemoveInfo(DefaultWeightedEdge edgeToRemove) { + //clone graph and remove edge + Graph improvedGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + graph.vertexSet().forEach(improvedGraph::addVertex); for (DefaultWeightedEdge weightedEdge : graph.edgeSet()) { - clonedGraph.addEdge(graph.getEdgeSource(weightedEdge), graph.getEdgeTarget(weightedEdge), weightedEdge); + improvedGraph.addEdge(graph.getEdgeSource(weightedEdge), graph.getEdgeTarget(weightedEdge), weightedEdge); } + improvedGraph.removeEdge(edgeToRemove); + + // Calculate new cycle count + int newCycleCount = new CircularReferenceChecker().getCycles(improvedGraph).size(); + + //calculate new graph statistics + double removedEdgeWeight = graph.getEdgeWeight(edgeToRemove); + double payoff = newCycleCount / removedEdgeWeight; + return new EdgeToRemoveInfo(edgeToRemove, (int) removedEdgeWeight, newCycleCount, payoff); + } + + /*public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) { List edgesToRemove = new ArrayList<>(); // capture impact of each edge on graph when removed for (DefaultWeightedEdge edge : edgesAboveDiagonal) { @@ -284,7 +345,7 @@ public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) // identify updated cycles and calculate updated graph information edgesToRemove.add(getEdgeToRemoveInfo( - edge, edgeInCyclesCount, avgCycleNodeCount, new CircularReferenceChecker().getCycles(clonedGraph))); + edge, edgeInCyclesCount, new CircularReferenceChecker().getCycles(clonedGraph))); // add the edge back for next iteration clonedGraph.addEdge(graph.getEdgeSource(edge), graph.getEdgeTarget(edge), edge); @@ -294,38 +355,134 @@ public List getImpactOfEdgesAboveDiagonalIfRemoved(int limit) edgesToRemove.sort(Comparator.comparing(EdgeToRemoveInfo::getPayoff)); Collections.reverse(edgesToRemove); return edgesToRemove; + }*/ + + public List getEdgesAboveDiagonal(Graph graph, List sortedActivities) { + List edgesAboveDiagonal = new ArrayList<>(); + for (int i = 0; i < sortedActivities.size(); i++) { + for (int j = i + 1; j < sortedActivities.size(); j++) { + // source / destination vertex was flipped after solution generation + // to correctly identify the vertex above the diagonal to remove + DefaultWeightedEdge edge = graph.getEdge(sortedActivities.get(i), sortedActivities.get(j)); + if (edge != null) { + edgesAboveDiagonal.add(edge); + } + } + } + + return edgesAboveDiagonal; + } + + private List orderVertices(Graph graph) { + List> sccs = findStronglyConnectedComponents(graph); + List sparseIntSortedActivities = topologicalSort(sccs, graph); + // reversing corrects rendering of the DSM + // with sources as rows and targets as columns + // was needed after AI solution was generated and iterated + Collections.reverse(sparseIntSortedActivities); + + return sparseIntSortedActivities; + } + + private List topologicalSort(List> sccs, Graph graph) { + List sortedActivities = new ArrayList<>(); + Set visited = new HashSet<>(); + + for (Set scc : sccs) { + for (String activity : scc) { + if (!visited.contains(activity)) { + topologicalSortUtil(activity, visited, sortedActivities, graph); + } + } + } + + Collections.reverse(sortedActivities); + return sortedActivities; + } + + private void topologicalSortUtil( + String activity, Set visited, List sortedActivities, Graph graph) { + visited.add(activity); + + for (String neighbor : Graphs.successorListOf(graph, activity)) { + if (!visited.contains(neighbor)) { + topologicalSortUtil(neighbor, visited, sortedActivities, graph); + } + } + + sortedActivities.add(activity); } - private EdgeToRemoveInfo getEdgeToRemoveInfo( - DefaultWeightedEdge edge, - int edgeInCyclesCount, - double currentAvgCycleNodeCount, - Map> cycles) { - // get the new number of cycles - int newCycleCount = cycles.size(); + private List> findStronglyConnectedComponents(Graph graph) { + KosarajuStrongConnectivityInspector kosaraju = + new KosarajuStrongConnectivityInspector<>(graph); + return kosaraju.stronglyConnectedSets(); + } - // calculate the average cycle node count - double newAverageCycleNodeCount = getAverageCycleNodeCount(cycles); + ///////////////////////////////////////////////////////// + // Sparse Int Graph implementation to find edge to remove + ///////////////////////////////////////////////////////// - // capture the what-if values - double edgeWeight = graph.getEdgeWeight(edge); + public List getImpactOfSparseEdgesAboveDiagonalIfRemoved() { + List sparseEdgesAboveDiagonal = getSparseEdgesAboveDiagonal(); + return sparseEdgesAboveDiagonal.stream() + .map(this::calculateSparseEdgeToRemoveInfo) + .sorted(Comparator.comparing(EdgeToRemoveInfo::getPayoff).thenComparing(EdgeToRemoveInfo::getRemovedEdgeWeight)) + .collect(Collectors.toList()); + } - double impact = (currentAvgCycleNodeCount - newAverageCycleNodeCount) / edgeWeight; - return new EdgeToRemoveInfo( - edge, edgeWeight, edgeInCyclesCount, newCycleCount, newAverageCycleNodeCount, impact); + private EdgeToRemoveInfo calculateSparseEdgeToRemoveInfo(Integer edgeToRemove) { + //clone graph and remove edge + int source = sparseGraph.getEdgeSource(edgeToRemove); + int target = sparseGraph.getEdgeTarget(edgeToRemove); + double weight = sparseGraph.getEdgeWeight(edgeToRemove); + Triple removedEdge = Triple.of(source, target, weight); + + List> updatedEdgeList = new ArrayList<>(sparseEdges); + updatedEdgeList.remove(removedEdge); + + SparseIntDirectedWeightedGraph improvedGraph = new SparseIntDirectedWeightedGraph(vertexCount, updatedEdgeList); + + // find edges above diagonal + List sortedSparseActivities = orderVertices(improvedGraph); + List updatedEdges = getSparseEdgesAboveDiagonal(improvedGraph, sortedSparseActivities); + + // calculate new graph statistics + int newEdgeCount = updatedEdges.size(); + double newEdgeWeightSum = updatedEdges.stream() + .mapToDouble(improvedGraph::getEdgeWeight).sum(); + DefaultWeightedEdge defaultWeightedEdge = + graph.getEdge(intToVertex.get(source), intToVertex.get(target)); + double payoff = (sumOfEdgeWeightsAboveDiagonal - newEdgeWeightSum) / weight; + return new EdgeToRemoveInfo(defaultWeightedEdge, (int) weight, newEdgeCount, payoff); } - public static double getAverageCycleNodeCount(Map> cycles) { - return cycles.values().stream() - .mapToInt(cycle -> cycle.vertexSet().size()) - .average() - .orElse(0.0); + private List orderVertices(SparseIntDirectedWeightedGraph sparseGraph) { + List> sccs = this.findStronglyConnectedSparseGraphComponents(sparseGraph); + List sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); + // reversing corrects rendering of the DSM + // with sources as rows and targets as columns + // was needed after AI solution was generated and iterated + Collections.reverse(sparseIntSortedActivities); + + return sparseIntSortedActivities; } - public double getAverageCycleNodeCount() { - return cycles.values().stream() - .mapToInt(cycle -> cycle.vertexSet().size()) - .average() - .orElse(0.0); + private List getSparseEdgesAboveDiagonal(SparseIntDirectedWeightedGraph sparseGraph, List sparseIntSortedActivities) { + List sparseEdgesAboveDiagonal = new ArrayList<>(); + + for (int i = 0; i < sparseIntSortedActivities.size(); i++) { + for (int j = i + 1; j < sparseIntSortedActivities.size(); j++) { + // source / destination vertex was flipped after solution generation + // to correctly identify the vertex above the diagonal to remove + Integer edge = sparseGraph.getEdge(sparseIntSortedActivities.get(i), sparseIntSortedActivities.get(j)); + + if (edge != null) { + sparseEdgesAboveDiagonal.add(edge); + } + } + } + + return sparseEdgesAboveDiagonal; } } diff --git a/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java b/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java index c16e0967..f284216b 100644 --- a/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java +++ b/dsm/src/main/java/org/hjug/dsm/EdgeToRemoveInfo.java @@ -6,9 +6,7 @@ @Data public class EdgeToRemoveInfo { private final DefaultWeightedEdge edge; - private final double edgeWeight; - private final int edgeInCycleCount; + private final int removedEdgeWeight; private final int newCycleCount; - private final double averageCycleNodeCount; private final double payoff; // impact / effort } diff --git a/dsm/src/main/java/org/hjug/dsm/OptimalBackEdgeRemover.java b/dsm/src/main/java/org/hjug/dsm/OptimalBackEdgeRemover.java new file mode 100644 index 00000000..598396a9 --- /dev/null +++ b/dsm/src/main/java/org/hjug/dsm/OptimalBackEdgeRemover.java @@ -0,0 +1,105 @@ +package org.hjug.dsm; + +import org.jgrapht.Graph; +import org.jgrapht.alg.cycle.CycleDetector; +import org.jgrapht.alg.cycle.JohnsonSimpleCycles; +import org.jgrapht.graph.AsSubgraph; + +import java.util.*; + +public class OptimalBackEdgeRemover { + private Graph graph; + + /** + * Constructor initializing with the target graph. + * @param graph The directed weighted graph to analyze + */ + public OptimalBackEdgeRemover(Graph graph) { + this.graph = graph; + } + + /** + * Finds the optimal back edge(s) to remove to move the graph closer to a DAG. + * @return A set of edges to remove + */ + public Set findOptimalBackEdgesToRemove() { + CycleDetector cycleDetector = new CycleDetector<>(graph); + + // If the graph is already acyclic, return empty set + if (!cycleDetector.detectCycles()) { + return Collections.emptySet(); + } + + // Find all cycles in the graph + JohnsonSimpleCycles cycleFinder = new JohnsonSimpleCycles<>(graph); + List> originalCycles = cycleFinder.findSimpleCycles(); + int originalCycleCount = originalCycles.size(); + + // Identify edges that are part of at least one cycle + Set edgesInCycles = new HashSet<>(); + for (List cycle : originalCycles) { + for (int i = 0; i < cycle.size(); i++) { + V source = cycle.get(i); + V target = cycle.get((i + 1) % cycle.size()); + E edge = graph.getEdge(source, target); + edgesInCycles.add(edge); + } + } + + // Calculate cycle elimination count for each edge + Map edgeCycleEliminationCount = new HashMap<>(); + for (E edge : edgesInCycles) { + // Create a subgraph without this edge + Graph subgraph = new AsSubgraph<>(graph, graph.vertexSet(), new HashSet<>(graph.edgeSet())); + subgraph.removeEdge(edge); + + // Calculate how many cycles would be eliminated + JohnsonSimpleCycles subgraphCycleFinder = new JohnsonSimpleCycles<>(subgraph); + List> remainingCycles = subgraphCycleFinder.findSimpleCycles(); + int cyclesEliminated = originalCycleCount - remainingCycles.size(); + + edgeCycleEliminationCount.put(edge, cyclesEliminated); + } + + // Find edges that eliminate the most cycles + int maxCycleElimination = 0; + List maxEliminationEdges = new ArrayList<>(); + + for (Map.Entry entry : edgeCycleEliminationCount.entrySet()) { + if (entry.getValue() > maxCycleElimination) { + maxCycleElimination = entry.getValue(); + maxEliminationEdges.clear(); + maxEliminationEdges.add(entry.getKey()); + } else if (entry.getValue() == maxCycleElimination) { + maxEliminationEdges.add(entry.getKey()); + } + } + + // If no cycles are eliminated (shouldn't happen), return empty set + if (maxEliminationEdges.isEmpty() || maxCycleElimination == 0) { + return Collections.emptySet(); + } + + // If multiple edges eliminate the same number of cycles, choose the one with the lowest weight + if (maxEliminationEdges.size() > 1) { + double minWeight = Double.MAX_VALUE; + List minWeightEdges = new ArrayList<>(); + + for (E edge : maxEliminationEdges) { + double weight = graph.getEdgeWeight(edge); + if (weight < minWeight) { + minWeight = weight; + minWeightEdges.clear(); + minWeightEdges.add(edge); + } else if (weight == minWeight) { + minWeightEdges.add(edge); + } + } + + return new HashSet<>(minWeightEdges); + } + + // Return the single edge that eliminates the most cycles + return new HashSet<>(maxEliminationEdges); + } +} diff --git a/dsm/src/main/java/org/hjug/dsm/SparseGraphCircularReferenceChecker.java b/dsm/src/main/java/org/hjug/dsm/SparseGraphCircularReferenceChecker.java new file mode 100644 index 00000000..926439ad --- /dev/null +++ b/dsm/src/main/java/org/hjug/dsm/SparseGraphCircularReferenceChecker.java @@ -0,0 +1,71 @@ +package org.hjug.dsm; + +import lombok.extern.slf4j.Slf4j; +import org.jgrapht.alg.cycle.CycleDetector; +import org.jgrapht.graph.AsSubgraph; +import org.jgrapht.opt.graph.sparse.SparseIntDirectedWeightedGraph; + +import java.util.HashMap; +import java.util.Map; + +@Slf4j +public class SparseGraphCircularReferenceChecker { + + private final Map> uniqueSubGraphs = new HashMap<>(); + + /** + * Detects cycles in the graph that is passed in + * and returns the unique cycles in the graph as a map of subgraphs + * + * @param graph + * @return a Map of unique cycles in the graph + */ + public Map> getCycles(SparseIntDirectedWeightedGraph graph) { + + if (!uniqueSubGraphs.isEmpty()) { + return uniqueSubGraphs; + } + + // use CycleDetector.findCycles()? + Map> cycles = detectCycles(graph); + + cycles.forEach((vertex, subGraph) -> { + int vertexCount = subGraph.vertexSet().size(); + int edgeCount = subGraph.edgeSet().size(); + + if (vertexCount > 1 && edgeCount > 1 && !isDuplicateSubGraph(subGraph, vertex)) { + uniqueSubGraphs.put(vertex, subGraph); + log.debug("Vertex: {} vertex count: {} edge count: {}", vertex, vertexCount, edgeCount); + } + }); + + return uniqueSubGraphs; + } + + private boolean isDuplicateSubGraph(AsSubgraph subGraph, Integer vertex) { + if (!uniqueSubGraphs.isEmpty()) { + for (AsSubgraph renderedSubGraph : uniqueSubGraphs.values()) { + if (renderedSubGraph.vertexSet().size() == subGraph.vertexSet().size() + && renderedSubGraph.edgeSet().size() + == subGraph.edgeSet().size() + && renderedSubGraph.vertexSet().contains(vertex)) { + return true; + } + } + } + + return false; + } + + private Map> detectCycles( + SparseIntDirectedWeightedGraph graph) { + Map> cyclesForEveryVertexMap = new HashMap<>(); + CycleDetector cycleDetector = new CycleDetector<>(graph); + cycleDetector.findCycles().forEach(v -> { + AsSubgraph subGraph = + new AsSubgraph<>(graph, cycleDetector.findCyclesContainingVertex(v)); + cyclesForEveryVertexMap.put(v, subGraph); + }); + return cyclesForEveryVertexMap; + } +} diff --git a/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java b/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java new file mode 100644 index 00000000..80fdfd6e --- /dev/null +++ b/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java @@ -0,0 +1,181 @@ +package org.hjug.dsm; + +import org.jgrapht.Graph; +import org.jgrapht.Graphs; +import org.jgrapht.alg.connectivity.KosarajuStrongConnectivityInspector; +import org.jgrapht.alg.util.Triple; +import org.jgrapht.graph.DefaultWeightedEdge; +import org.jgrapht.opt.graph.sparse.SparseIntDirectedWeightedGraph; + +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ConcurrentSkipListSet; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +class SparseIntDWGEdgeRemovalCalculator { + private final Graph graph; + SparseIntDirectedWeightedGraph sparseGraph; + List> sparseEdges; + List sparseEdgesAboveDiagonal; + private final double sumOfEdgeWeightsAboveDiagonal; + int vertexCount; + Map vertexToInt; + Map intToVertex; + + + SparseIntDWGEdgeRemovalCalculator( + Graph graph, + SparseIntDirectedWeightedGraph sparseGraph, + List> sparseEdges, + List sparseEdgesAboveDiagonal, + double sumOfEdgeWeightsAboveDiagonal, + int vertexCount, + Map vertexToInt, + Map intToVertex) { + //TODO: Use concurrent types where possible + this.graph = graph; + this.sparseGraph = sparseGraph; + this.sparseEdges = new CopyOnWriteArrayList<>(sparseEdges); + this.sparseEdgesAboveDiagonal = new CopyOnWriteArrayList<>(sparseEdgesAboveDiagonal); + this.sumOfEdgeWeightsAboveDiagonal = sumOfEdgeWeightsAboveDiagonal; + this.vertexCount = vertexCount; + this.vertexToInt = new ConcurrentHashMap<>(vertexToInt); + this.intToVertex = new ConcurrentHashMap<>(intToVertex); + + } + + public List getImpactOfSparseEdgesAboveDiagonalIfRemoved() { + return sparseEdgesAboveDiagonal.parallelStream() + .map(this::calculateSparseEdgeToRemoveInfo) + .sorted(Comparator.comparing(EdgeToRemoveInfo::getPayoff).thenComparing(EdgeToRemoveInfo::getRemovedEdgeWeight)) + .collect(Collectors.toList()); + } + + private EdgeToRemoveInfo calculateSparseEdgeToRemoveInfo(Integer edgeToRemove) { + //clone graph and remove edge + int source = sparseGraph.getEdgeSource(edgeToRemove); + int target = sparseGraph.getEdgeTarget(edgeToRemove); + double weight = sparseGraph.getEdgeWeight(edgeToRemove); + Triple removedEdge = Triple.of(source, target, weight); + + List> tempUpdatedEdgeList = new ArrayList<>(sparseEdges); + tempUpdatedEdgeList.remove(removedEdge); + List> updatedEdgeList = new CopyOnWriteArrayList<>(tempUpdatedEdgeList); + + SparseIntDirectedWeightedGraph improvedGraph = new SparseIntDirectedWeightedGraph(vertexCount, updatedEdgeList); + + // find edges above diagonal + List sortedSparseVertices = orderVertices(improvedGraph); + List updatedEdges = getSparseEdgesAboveDiagonal(improvedGraph, sortedSparseVertices); + + // calculate new graph statistics + int newEdgeCount = updatedEdges.size(); + double newEdgeWeightSum = updatedEdges.stream() + .mapToDouble(improvedGraph::getEdgeWeight).sum(); + DefaultWeightedEdge defaultWeightedEdge = + graph.getEdge(intToVertex.get(source), intToVertex.get(target)); + double payoff = (sumOfEdgeWeightsAboveDiagonal - newEdgeWeightSum) / weight; + return new EdgeToRemoveInfo(defaultWeightedEdge, (int) weight, newEdgeCount, payoff); + } + + private List orderVertices(SparseIntDirectedWeightedGraph sparseGraph) { + List> sccs = new CopyOnWriteArrayList<>(findStronglyConnectedSparseGraphComponents(sparseGraph)); + List sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); + // reversing corrects rendering of the DSM + // with sources as rows and targets as columns + // was needed after AI solution was generated and iterated + Collections.reverse(sparseIntSortedActivities); + + return new CopyOnWriteArrayList<>(sparseIntSortedActivities); + } + + /** + * Kosaraju SCC detector avoids stack overflow. + * It is used by JGraphT's CycleDetector, and makes sense to use it here as well for consistency + * + * @param graph + * @return + */ + private List> findStronglyConnectedSparseGraphComponents(Graph graph) { + KosarajuStrongConnectivityInspector kosaraju = + new KosarajuStrongConnectivityInspector<>(graph); + return kosaraju.stronglyConnectedSets(); + } + + private List topologicalSortSparseGraph(List> sccs, Graph graph) { + List sortedActivities = new ArrayList<>(); + Set visited = new HashSet<>(); + + sccs.parallelStream() + .flatMap(Set::parallelStream) + .filter(activity -> !visited.contains(activity)) + .forEach(activity -> topologicalSortUtilSparseGraph(activity, visited, sortedActivities, graph)); + + + Collections.reverse(sortedActivities); + return sortedActivities; + } + + private void topologicalSortUtilSparseGraph( + Integer activity, Set visited, List sortedActivities, Graph graph) { + visited.add(activity); + + for (Integer neighbor : Graphs.successorListOf(graph, activity)) { + if (!visited.contains(neighbor)) { + topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph); + } + } + + sortedActivities.add(activity); + } + + private List getSparseEdgesAboveDiagonal(SparseIntDirectedWeightedGraph sparseGraph, List sortedActivities) { + ConcurrentLinkedQueue sparseEdgesAboveDiagonal = new ConcurrentLinkedQueue<>(); + + int size = sortedActivities.size(); + IntStream.range(0, size).parallel().forEach(i -> { + for (int j = i + 1; j < size; j++) { + Integer edge = sparseGraph.getEdge( + sortedActivities.get(i), + sortedActivities.get(j) + ); + if (edge != null) { + sparseEdgesAboveDiagonal.add(edge); + } + } + }); + + return new ArrayList<>(sparseEdgesAboveDiagonal); + } + + private List topologicalParallelSortSparseGraph(List> sccs, Graph graph) { + ConcurrentLinkedQueue sortedActivities = new ConcurrentLinkedQueue<>(); + Set visited = new ConcurrentSkipListSet<>(); + + sccs.stream() + .flatMap(Set::parallelStream) + .filter(activity -> !visited.contains(activity)) + .forEach(activity -> topologicalSortUtilSparseGraph(activity, visited, sortedActivities, graph)); + + ArrayList sortedActivitiesList = new ArrayList<>(sortedActivities); + Collections.reverse(sortedActivitiesList); + return sortedActivitiesList; + } + + private void topologicalSortUtilSparseGraph( + Integer activity, Set visited, ConcurrentLinkedQueue sortedActivities, Graph graph) { + visited.add(activity); + + for (Integer neighbor : Graphs.successorListOf(graph, activity)) { + if (!visited.contains(neighbor)) { + topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph); + } + } + + sortedActivities.add(activity); + } + +} diff --git a/dsm/src/test/java/org/hjug/dsm/DSMTest.java b/dsm/src/test/java/org/hjug/dsm/DSMTest.java index 4e871eb1..bdc3242f 100644 --- a/dsm/src/test/java/org/hjug/dsm/DSMTest.java +++ b/dsm/src/test/java/org/hjug/dsm/DSMTest.java @@ -129,6 +129,5 @@ void getImpactOfEdgesAboveDiagonalIfRemoved() { assertEquals("(H : E)", infos.get(0).getEdge().toString()); assertEquals(2, infos.get(0).getNewCycleCount()); - assertEquals(4.5, infos.get(0).getAverageCycleNodeCount()); } } diff --git a/dsm/src/test/java/org/hjug/dsm/OptimalBackEdgeRemoverTest.java b/dsm/src/test/java/org/hjug/dsm/OptimalBackEdgeRemoverTest.java new file mode 100644 index 00000000..5eb9a0d8 --- /dev/null +++ b/dsm/src/test/java/org/hjug/dsm/OptimalBackEdgeRemoverTest.java @@ -0,0 +1,102 @@ +package org.hjug.dsm; + +import org.jgrapht.Graph; +import org.jgrapht.graph.DefaultWeightedEdge; +import org.jgrapht.graph.SimpleDirectedWeightedGraph; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.*; + +class OptimalBackEdgeRemoverTest { + + @Test + void noOptimalEdge() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + assertTrue(optimalEdges.isEmpty()); + } + + + @Test + void oneBackEdge() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + classReferencesGraph.addEdge("C", "A"); + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + // all are considered back edges since this is a cycle + assertEquals(3, optimalEdges.size()); + } + + @Test + void twoBackEdges() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addVertex("D"); + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + classReferencesGraph.addEdge("C", "D"); + classReferencesGraph.addEdge("C", "A"); // back edge + classReferencesGraph.addEdge("D", "A"); // back edge + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + assertEquals(2, optimalEdges.size()); + } + + @Test + void multi() { + Graph classReferencesGraph = new SimpleDirectedWeightedGraph<>(DefaultWeightedEdge.class); + classReferencesGraph.addVertex("A"); + classReferencesGraph.addVertex("B"); + classReferencesGraph.addVertex("C"); + classReferencesGraph.addVertex("D"); + + // Cycle 1 + classReferencesGraph.addEdge("A", "B"); + classReferencesGraph.addEdge("B", "C"); + classReferencesGraph.addEdge("C", "D"); + classReferencesGraph.addEdge("B", "A"); // Adding a cycle + classReferencesGraph.addEdge("C", "A"); // Adding a cycle + classReferencesGraph.addEdge("D", "A"); // Adding a cycle + + // Cycle 2 + classReferencesGraph.addVertex("E"); + classReferencesGraph.addVertex("F"); + classReferencesGraph.addVertex("G"); + classReferencesGraph.addVertex("H"); + classReferencesGraph.addEdge("E", "F"); + classReferencesGraph.addEdge("F", "G"); + classReferencesGraph.addEdge("G", "H"); + classReferencesGraph.addEdge("H", "E"); // create cycle + + classReferencesGraph.addEdge("A", "E"); + classReferencesGraph.addEdge("E", "A"); // create cycle between cycles + + OptimalBackEdgeRemover remover = new OptimalBackEdgeRemover(classReferencesGraph); + Set optimalEdges = remover.findOptimalBackEdgesToRemove(); + + assertEquals(1, optimalEdges.size()); + assertEquals("E:A", new ArrayList<>(optimalEdges).get(0).toString()); + } +} \ No newline at end of file diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java index b96d16f5..d3d3d7aa 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java @@ -14,7 +14,6 @@ import java.util.Locale; import java.util.Optional; import lombok.extern.slf4j.Slf4j; -import org.hjug.cbc.CostBenefitCalculator; import org.hjug.cbc.CycleRanker; import org.hjug.cbc.RankedCycle; import org.hjug.cbc.RankedDisharmony; @@ -193,14 +192,14 @@ public StringBuilder generateReport( List rankedGodClassDisharmonies = List.of(); List rankedCBODisharmonies = List.of(); log.info("Identifying Object Oriented Disharmonies"); - try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir)) { - costBenefitCalculator.runPmdAnalysis(); - rankedGodClassDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues(); - rankedCBODisharmonies = costBenefitCalculator.calculateCBOCostBenefitValues(); - } catch (Exception e) { - log.error("Error running analysis."); - throw new RuntimeException(e); - } + // try (CostBenefitCalculator costBenefitCalculator = new CostBenefitCalculator(projectBaseDir)) { + // costBenefitCalculator.runPmdAnalysis(); + // rankedGodClassDisharmonies = costBenefitCalculator.calculateGodClassCostBenefitValues(); + // rankedCBODisharmonies = costBenefitCalculator.calculateCBOCostBenefitValues(); + // } catch (Exception e) { + // log.error("Error running analysis."); + // throw new RuntimeException(e); + // } CycleRanker cycleRanker = new CycleRanker(projectBaseDir); List rankedCycles = List.of(); @@ -216,7 +215,7 @@ public StringBuilder generateReport( edgesAboveDiagonal = dsm.getEdgesAboveDiagonal(); log.info("Performing edge removal what-if analysis"); - List edgeToRemoveInfos = dsm.getImpactOfEdgesAboveDiagonalIfRemoved(edgeAnalysisCount); + List edgeToRemoveInfos = dsm.getImpactOfSparseEdgesAboveDiagonalIfRemoved(); if (edgeToRemoveInfos.isEmpty() && rankedGodClassDisharmonies.isEmpty() @@ -313,10 +312,6 @@ private String renderEdgeToRemoveInfos(List edges) { .append("Current Cycle Count: ") .append(dsm.getCycles().size()) .append("
\n"); - stringBuilder - .append("Current Average Cycle Node Count: ") - .append(dsm.getAverageCycleNodeCount()) - .append("
\n"); stringBuilder .append("Current Total Back Edge Count: ") .append(dsm.getEdgesAboveDiagonal().size()) @@ -424,22 +419,17 @@ private String[] getEdgesToRemoveInfoTableHeadings() { return new String[] { "Edge", "Edge Weight", - "In # of Cycles", "New Cycle Count", - "New Avg Cycle Node Count", "Avg Node Δ ÷ Effort" }; } private String[] getEdgeToRemoveInfos(EdgeToRemoveInfo edgeToRemoveInfo) { return new String[] { - // "Edge", "Edge Weight", "In # of Cycles", "New Cycle Count", "New Avg Cycle Node Count", "Avg Node Count / - // Effort" + // "Edge", "Edge Weight", "In # of Cycles", "New Back Edge Count", "New Back Edge Weight Sum", "Payoff" renderEdge(edgeToRemoveInfo.getEdge()), - String.valueOf((int) edgeToRemoveInfo.getEdgeWeight()), - String.valueOf(edgeToRemoveInfo.getEdgeInCycleCount()), + String.valueOf(edgeToRemoveInfo.getRemovedEdgeWeight()), String.valueOf(edgeToRemoveInfo.getNewCycleCount()), - String.valueOf(edgeToRemoveInfo.getAverageCycleNodeCount()), String.valueOf(edgeToRemoveInfo.getPayoff()) }; } @@ -450,8 +440,7 @@ private String[] getRankedCycleSummaryData(RankedCycle rankedCycle, StringBuilde getClassName(rankedCycle.getCycleName()), rankedCycle.getPriority().toString(), String.valueOf(rankedCycle.getCycleNodes().size()), - String.valueOf(rankedCycle.getEdgeSet().size()) // , - // edgesToCut.toString() + String.valueOf(rankedCycle.getEdgeSet().size()) }; } From e1372cc4e755f71b779bf3ff5f9e467866dd7617 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Wed, 25 Jun 2025 20:03:39 -0500 Subject: [PATCH 2/2] Committing, but will remove much of the work that has been done to take a different approach. --- dsm/src/main/java/org/hjug/dsm/DSM.java | 2 ++ .../SparseIntDWGEdgeRemovalCalculator.java | 15 +++++----- pom.xml | 2 +- .../report/SimpleHtmlReport.java | 28 +++++++++---------- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/dsm/src/main/java/org/hjug/dsm/DSM.java b/dsm/src/main/java/org/hjug/dsm/DSM.java index 0dedceec..bfd5ffa5 100644 --- a/dsm/src/main/java/org/hjug/dsm/DSM.java +++ b/dsm/src/main/java/org/hjug/dsm/DSM.java @@ -278,6 +278,8 @@ void printDSM(Graph graph, List sortedActiv } } + // TODO: Delete all code below this line + // Will be superseded by Minimum Feedback Arc + Vertex calculations ///////////////////////////////////////////////////////// // "Standard" Graph implementation to find edge to remove ///////////////////////////////////////////////////////// diff --git a/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java b/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java index 80fdfd6e..01d6aa24 100644 --- a/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java +++ b/dsm/src/main/java/org/hjug/dsm/SparseIntDWGEdgeRemovalCalculator.java @@ -15,6 +15,7 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +// TODO: Delete class SparseIntDWGEdgeRemovalCalculator { private final Graph graph; SparseIntDirectedWeightedGraph sparseGraph; @@ -35,7 +36,6 @@ class SparseIntDWGEdgeRemovalCalculator { int vertexCount, Map vertexToInt, Map intToVertex) { - //TODO: Use concurrent types where possible this.graph = graph; this.sparseGraph = sparseGraph; this.sparseEdges = new CopyOnWriteArrayList<>(sparseEdges); @@ -83,7 +83,8 @@ private EdgeToRemoveInfo calculateSparseEdgeToRemoveInfo(Integer edgeToRemove) { private List orderVertices(SparseIntDirectedWeightedGraph sparseGraph) { List> sccs = new CopyOnWriteArrayList<>(findStronglyConnectedSparseGraphComponents(sparseGraph)); - List sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); +// List sparseIntSortedActivities = topologicalSortSparseGraph(sccs, sparseGraph); + List sparseIntSortedActivities = topologicalParallelSortSparseGraph(sccs, sparseGraph); // reversing corrects rendering of the DSM // with sources as rows and targets as columns // was needed after AI solution was generated and iterated @@ -155,7 +156,7 @@ private List topologicalParallelSortSparseGraph(List> sccs ConcurrentLinkedQueue sortedActivities = new ConcurrentLinkedQueue<>(); Set visited = new ConcurrentSkipListSet<>(); - sccs.stream() + sccs.parallelStream() .flatMap(Set::parallelStream) .filter(activity -> !visited.contains(activity)) .forEach(activity -> topologicalSortUtilSparseGraph(activity, visited, sortedActivities, graph)); @@ -169,11 +170,9 @@ private void topologicalSortUtilSparseGraph( Integer activity, Set visited, ConcurrentLinkedQueue sortedActivities, Graph graph) { visited.add(activity); - for (Integer neighbor : Graphs.successorListOf(graph, activity)) { - if (!visited.contains(neighbor)) { - topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph); - } - } + Graphs.successorListOf(graph, activity).parallelStream() + .filter(neighbor -> !visited.contains(neighbor)) + .forEach(neighbor -> topologicalSortUtilSparseGraph(neighbor, visited, sortedActivities, graph)); sortedActivities.add(activity); } diff --git a/pom.xml b/pom.xml index 3702fe8b..aa6ac31f 100644 --- a/pom.xml +++ b/pom.xml @@ -390,7 +390,7 @@ com.diffplug.spotless spotless-maven-plugin - 2.30.0 + 2.44.5 diff --git a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java index d3d3d7aa..15888825 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/SimpleHtmlReport.java @@ -215,10 +215,10 @@ public StringBuilder generateReport( edgesAboveDiagonal = dsm.getEdgesAboveDiagonal(); log.info("Performing edge removal what-if analysis"); - List edgeToRemoveInfos = dsm.getImpactOfSparseEdgesAboveDiagonalIfRemoved(); +// List edgeToRemoveInfos = dsm.getImpactOfSparseEdgesAboveDiagonalIfRemoved(); - if (edgeToRemoveInfos.isEmpty() - && rankedGodClassDisharmonies.isEmpty() + if (/*edgeToRemoveInfos.isEmpty() + &&*/ rankedGodClassDisharmonies.isEmpty() && rankedCBODisharmonies.isEmpty() && rankedCycles.isEmpty()) { stringBuilder @@ -232,10 +232,10 @@ public StringBuilder generateReport( return stringBuilder; } - if (!edgeToRemoveInfos.isEmpty()) { - stringBuilder.append("Back Edges\n"); - stringBuilder.append("
\n"); - } +// if (!edgeToRemoveInfos.isEmpty()) { +// stringBuilder.append("Back Edges\n"); +// stringBuilder.append("
\n"); +// } if (!rankedGodClassDisharmonies.isEmpty()) { stringBuilder.append("God Classes\n"); @@ -259,13 +259,13 @@ public StringBuilder generateReport( // Display impact of each edge if removed stringBuilder.append("
\n"); - String edgeInfos = renderEdgeToRemoveInfos(edgeToRemoveInfos); - - if (!edgeToRemoveInfos.isEmpty()) { - stringBuilder.append(edgeInfos); - stringBuilder.append(renderGithubButtons()); - stringBuilder.append("
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n"); - } +// String edgeInfos = renderEdgeToRemoveInfos(edgeToRemoveInfos); +// +// if (!edgeToRemoveInfos.isEmpty()) { +// stringBuilder.append(edgeInfos); +// stringBuilder.append(renderGithubButtons()); +// stringBuilder.append("
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n" + "
\n"); +// } if (!rankedGodClassDisharmonies.isEmpty()) { final String[] godClassTableHeadings =