Skip to content
This repository was archived by the owner on Jun 18, 2026. It is now read-only.

Commit 082f816

Browse files
refactor(chordal): eliminate redundant adjacency map construction in public API
Each public method (testChordality, optimalColoring, maximumClique, allMaximalCliques, computeFillIn) independently called GraphUtils.buildAdjacencyMap(graph), which walks every edge to build the neighbor-set map. For composite calls like minimalSeparators() (which called allMaximalCliques + buildCliqueTree, each of which called testChordality internally), the adjacency map was rebuilt up to 9 times for a single analysis. Refactored to: - Add overloaded maximumCardinalitySearch(graph, adj) that accepts a pre-computed adjacency map - Public methods now compute adj once and delegate to existing private helpers (verifyChordalityFromMCS, colorFromPEO, maxCliqueFromPEO, maximalCliquesFromPEO, fillInFromMCS) - minimalSeparators now reuses cliques via buildCliqueTreeFromCliques instead of re-running allMaximalCliques through buildCliqueTree Net result: -150 lines of duplicated logic, single adjacency map construction per public method call. For the analyze() entry point (which already used the private helpers), the MCS overload avoids one redundant adj rebuild.
1 parent b6d83b7 commit 082f816

1 file changed

Lines changed: 35 additions & 185 deletions

File tree

Gvisual/src/gvisual/ChordalGraphAnalyzer.java

Lines changed: 35 additions & 185 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,15 @@ public String toTextReport() {
193193
* @return MCS ordering (last eliminated first)
194194
*/
195195
public static List<String> maximumCardinalitySearch(Graph<String, Edge> graph) {
196+
return maximumCardinalitySearch(graph, GraphUtils.buildAdjacencyMap(graph));
197+
}
198+
199+
/**
200+
* MCS using a pre-computed adjacency map, avoiding redundant graph traversals
201+
* when the caller already has the adjacency structure.
202+
*/
203+
static List<String> maximumCardinalitySearch(Graph<String, Edge> graph,
204+
Map<String, Set<String>> adj) {
196205
if (graph == null) return Collections.emptyList();
197206
Collection<String> vertices = graph.getVertices();
198207
if (vertices == null || vertices.isEmpty()) return Collections.emptyList();
@@ -206,7 +215,6 @@ public static List<String> maximumCardinalitySearch(Graph<String, Edge> graph) {
206215
}
207216

208217
List<String> ordering = new ArrayList<>(n);
209-
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
210218

211219
for (int i = 0; i < n; i++) {
212220
// Pick vertex with maximum weight among remaining
@@ -252,45 +260,10 @@ public static ChordalityResult testChordality(Graph<String, Edge> graph) {
252260
return new ChordalityResult(true, Collections.<String>emptyList(), null);
253261
}
254262

255-
List<String> mcsOrder = maximumCardinalitySearch(graph);
256263
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
257-
258-
// Position in ordering
259-
Map<String, Integer> pos = new HashMap<>();
260-
for (int i = 0; i < mcsOrder.size(); i++) {
261-
pos.put(mcsOrder.get(i), i);
262-
}
263-
264-
// Verify PEO: for each vertex v at position i,
265-
// its neighbors with position > i must form a clique
266-
for (int i = 0; i < mcsOrder.size(); i++) {
267-
String v = mcsOrder.get(i);
268-
Set<String> nbrs = adj.get(v);
269-
if (nbrs == null) continue;
270-
271-
List<String> laterNeighbors = new ArrayList<>();
272-
for (String nb : nbrs) {
273-
if (pos.get(nb) > i) {
274-
laterNeighbors.add(nb);
275-
}
276-
}
277-
278-
// Check all pairs of later neighbors are adjacent
279-
for (int a = 0; a < laterNeighbors.size(); a++) {
280-
for (int b = a + 1; b < laterNeighbors.size(); b++) {
281-
String u = laterNeighbors.get(a);
282-
String w = laterNeighbors.get(b);
283-
Set<String> uNbrs = adj.get(u);
284-
if (uNbrs == null || !uNbrs.contains(w)) {
285-
// Not chordal — find a chordless cycle
286-
List<String> cycle = findChordlessCycle(adj, v, u, w);
287-
return new ChordalityResult(false, mcsOrder, cycle);
288-
}
289-
}
290-
}
291-
}
292-
293-
return new ChordalityResult(true, mcsOrder, null);
264+
List<String> mcsOrder = maximumCardinalitySearch(graph, adj);
265+
Map<String, Integer> pos = buildPositionMap(mcsOrder);
266+
return verifyChordalityFromMCS(mcsOrder, adj, pos);
294267
}
295268

296269
/**
@@ -354,36 +327,9 @@ public static ColoringResult optimalColoring(Graph<String, Edge> graph) {
354327
return new ColoringResult(Collections.<String, Integer>emptyMap(), 0);
355328
}
356329

357-
ChordalityResult cr = testChordality(graph);
358-
List<String> peo = cr.getPeo();
359-
360-
// Greedy color in reverse PEO order (last eliminated first)
361-
List<String> reversed = new ArrayList<>(peo);
362-
Collections.reverse(reversed);
363-
364330
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
365-
Map<String, Integer> colors = new LinkedHashMap<>();
366-
int maxColor = 0;
367-
368-
for (String v : reversed) {
369-
// Find colors used by already-colored neighbors
370-
Set<Integer> usedColors = new HashSet<>();
371-
Set<String> nbrs = adj.get(v);
372-
if (nbrs != null) {
373-
for (String nb : nbrs) {
374-
if (colors.containsKey(nb)) {
375-
usedColors.add(colors.get(nb));
376-
}
377-
}
378-
}
379-
// Assign smallest available color
380-
int c = 0;
381-
while (usedColors.contains(c)) c++;
382-
colors.put(v, c);
383-
if (c > maxColor) maxColor = c;
384-
}
385-
386-
return new ColoringResult(colors, maxColor + 1);
331+
List<String> peo = maximumCardinalitySearch(graph, adj);
332+
return colorFromPEO(peo, adj);
387333
}
388334

389335
// ── Maximum clique (chordal) ─────────────────────────────────────────
@@ -401,40 +347,23 @@ public static Set<String> maximumClique(Graph<String, Edge> graph) {
401347
return Collections.emptySet();
402348
}
403349

404-
ChordalityResult cr = testChordality(graph);
405-
if (!cr.isChordal()) {
406-
// Fallback: find largest clique via enumeration (slow for non-chordal)
407-
return findMaxCliqueGreedy(graph);
408-
}
409-
410-
List<String> peo = cr.getPeo();
411-
Map<String, Integer> pos = new HashMap<>();
412-
for (int i = 0; i < peo.size(); i++) pos.put(peo.get(i), i);
413-
414350
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
351+
List<String> mcsOrder = maximumCardinalitySearch(graph, adj);
352+
Map<String, Integer> pos = buildPositionMap(mcsOrder);
353+
ChordalityResult cr = verifyChordalityFromMCS(mcsOrder, adj, pos);
415354

416-
Set<String> best = new LinkedHashSet<>();
417-
for (int i = 0; i < peo.size(); i++) {
418-
String v = peo.get(i);
419-
Set<String> clique = new LinkedHashSet<>();
420-
clique.add(v);
421-
Set<String> nbrs = adj.get(v);
422-
if (nbrs != null) {
423-
for (String nb : nbrs) {
424-
if (pos.get(nb) > i) {
425-
clique.add(nb);
426-
}
427-
}
428-
}
429-
if (clique.size() > best.size()) {
430-
best = clique;
431-
}
355+
if (!cr.isChordal()) {
356+
return findMaxCliqueGreedy(graph, adj);
432357
}
433-
return best;
358+
return maxCliqueFromPEO(mcsOrder, pos, adj);
434359
}
435360

436361
private static Set<String> findMaxCliqueGreedy(Graph<String, Edge> graph) {
437-
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
362+
return findMaxCliqueGreedy(graph, GraphUtils.buildAdjacencyMap(graph));
363+
}
364+
365+
private static Set<String> findMaxCliqueGreedy(Graph<String, Edge> graph,
366+
Map<String, Set<String>> adj) {
438367
// Sort vertices by degree descending
439368
List<String> sorted = new ArrayList<>(graph.getVertices());
440369
Collections.sort(sorted, (String a, String b) -> {
@@ -472,51 +401,10 @@ public static List<Set<String>> allMaximalCliques(Graph<String, Edge> graph) {
472401
return Collections.emptyList();
473402
}
474403

475-
ChordalityResult cr = testChordality(graph);
476-
List<String> peo = cr.getPeo();
477-
Map<String, Integer> pos = new HashMap<>();
478-
for (int i = 0; i < peo.size(); i++) pos.put(peo.get(i), i);
479-
480404
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
481-
482-
List<Set<String>> cliques = new ArrayList<>();
483-
Set<Set<String>> seen = new HashSet<>();
484-
485-
for (int i = 0; i < peo.size(); i++) {
486-
String v = peo.get(i);
487-
Set<String> clique = new TreeSet<>();
488-
clique.add(v);
489-
Set<String> nbrs = adj.get(v);
490-
if (nbrs != null) {
491-
for (String nb : nbrs) {
492-
if (pos.get(nb) > i) {
493-
clique.add(nb);
494-
}
495-
}
496-
}
497-
if (!seen.contains(clique)) {
498-
// Check maximality — not a subset of any existing
499-
boolean maximal = true;
500-
for (Set<String> existing : cliques) {
501-
if (existing.containsAll(clique)) {
502-
maximal = false;
503-
break;
504-
}
505-
}
506-
if (maximal) {
507-
// Remove any existing cliques that are subsets of this one
508-
Iterator<Set<String>> it = cliques.iterator();
509-
while (it.hasNext()) {
510-
if (clique.containsAll(it.next())) {
511-
it.remove();
512-
}
513-
}
514-
cliques.add(clique);
515-
seen.add(clique);
516-
}
517-
}
518-
}
519-
return cliques;
405+
List<String> peo = maximumCardinalitySearch(graph, adj);
406+
Map<String, Integer> pos = buildPositionMap(peo);
407+
return maximalCliquesFromPEO(peo, pos, adj);
520408
}
521409

522410
// ── Clique tree (junction tree) ──────────────────────────────────────
@@ -601,50 +489,12 @@ public static FillInResult computeFillIn(Graph<String, Edge> graph) {
601489
if (graph == null || graph.getVertexCount() == 0) {
602490
return new FillInResult(Collections.<String[]>emptyList());
603491
}
604-
605-
List<String> mcsOrder = maximumCardinalitySearch(graph);
606-
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
607-
608-
// Make a mutable copy of adjacency for fill-in
609-
Map<String, Set<String>> augmented = new HashMap<>();
610-
for (Map.Entry<String, Set<String>> e : adj.entrySet()) {
611-
augmented.put(e.getKey(), new HashSet<>(e.getValue()));
612-
}
613-
614-
Map<String, Integer> pos = new HashMap<>();
615-
for (int i = 0; i < mcsOrder.size(); i++) pos.put(mcsOrder.get(i), i);
616-
617-
List<String[]> fillEdges = new ArrayList<>();
618-
619-
for (int i = 0; i < mcsOrder.size(); i++) {
620-
String v = mcsOrder.get(i);
621-
Set<String> nbrs = augmented.get(v);
622-
if (nbrs == null) continue;
623-
624-
List<String> laterNeighbors = new ArrayList<>();
625-
for (String nb : nbrs) {
626-
if (pos.get(nb) > i) {
627-
laterNeighbors.add(nb);
628-
}
629-
}
630-
631-
for (int a = 0; a < laterNeighbors.size(); a++) {
632-
for (int b = a + 1; b < laterNeighbors.size(); b++) {
633-
String u = laterNeighbors.get(a);
634-
String w = laterNeighbors.get(b);
635-
if (!augmented.get(u).contains(w)) {
636-
// Add fill Edge
637-
augmented.get(u).add(w);
638-
augmented.get(w).add(u);
639-
String first = u.compareTo(w) < 0 ? u : w;
640-
String second = u.compareTo(w) < 0 ? w : u;
641-
fillEdges.add(new String[]{first, second});
642-
}
643-
}
644-
}
645492
}
646493

647-
return new FillInResult(fillEdges);
494+
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
495+
List<String> mcsOrder = maximumCardinalitySearch(graph, adj);
496+
Map<String, Integer> pos = buildPositionMap(mcsOrder);
497+
return fillInFromMCS(mcsOrder, pos, adj);
648498
}
649499

650500
// ── Minimum separators ───────────────────────────────────────────────
@@ -658,7 +508,7 @@ public static FillInResult computeFillIn(Graph<String, Edge> graph) {
658508
*/
659509
public static List<Set<String>> minimalSeparators(Graph<String, Edge> graph) {
660510
List<Set<String>> cliques = allMaximalCliques(graph);
661-
List<CliqueTreeNode> tree = buildCliqueTree(graph);
511+
List<CliqueTreeNode> tree = buildCliqueTreeFromCliques(cliques);
662512
if (tree.size() <= 1) return Collections.emptyList();
663513

664514
Set<Set<String>> separators = new LinkedHashSet<>();
@@ -702,7 +552,7 @@ public static List<Set<String>> eliminationCliques(Graph<String, Edge> graph,
702552
List<String> order) {
703553
if (graph == null || order == null) return Collections.emptyList();
704554

705-
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
555+
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph); // needed: mutable copy below
706556
// Mutable copy
707557
Map<String, Set<String>> remaining = new HashMap<>();
708558
for (Map.Entry<String, Set<String>> e : adj.entrySet()) {
@@ -765,7 +615,7 @@ public static ChordalReport analyze(Graph<String, Edge> graph) {
765615

766616
// Compute expensive structures once
767617
Map<String, Set<String>> adj = GraphUtils.buildAdjacencyMap(graph);
768-
List<String> mcsOrder = maximumCardinalitySearch(graph);
618+
List<String> mcsOrder = maximumCardinalitySearch(graph, adj);
769619
Map<String, Integer> pos = buildPositionMap(mcsOrder);
770620

771621
// Test chordality using pre-computed data

0 commit comments

Comments
 (0)