Add kamada_kawai_layout function#1583
Conversation
Adds rustworkx.kamada_kawai_layout, supporting both PyGraph and PyDiGraph inputs. Implements the original Kamada-Kawai (1989) algorithm: an outer loop selects the node with the largest partial-gradient norm and an inner loop applies a 2D Newton step against the local Hessian until convergence. Disconnected graphs are laid out by running Kamada-Kawai independently on each connected component and packing the components in a horizontal row, which avoids the visual collapse seen with single-objective Kamada-Kawai on disconnected inputs. Directed graphs have their distance matrix symmetrised before optimising (Kamada-Kawai is fundamentally undirected). Closes Qiskit#438
|
|
|
|
IvanIsCoding
left a comment
IvanIsCoding
left a comment
There was a problem hiding this comment.
This is a great start, I just left some comments about:
- Directed graphs
- Some conventions followed by rustworkx
This is very close to getting merged, it would be a fantastic addition to 0.18.0.
n.b: I was on vacation, that was the reason behind the delay of the feedback. this is a quality contribution.
| for j in (i + 1)..n_bound { | ||
| let d_ij = matrix[i * n_bound + j]; | ||
| let d_ji = matrix[j * n_bound + i]; | ||
| let d = d_ij.min(d_ji); |
There was a problem hiding this comment.
So this line is incorrect and the counter example are weakly connected graphs.
My suggestion: https://www.rustworkx.org/apiref/rustworkx.PyDiGraph.to_undirected.html#rustworkx-pydigraph-to-undirected, call this and pass it to the Dijkstra call as an undirected graph.
https://www.rustworkx.org/apiref/rustworkx.weakly_connected_components.html has an example of a weakly connected graph. Try seeing what the layout looks like for graph with edges like[(0, 1), (2, 1)] in this code vs NetworkX's code
There was a problem hiding this comment.
now builds an undirected StableUnGraph from the source topology and runs Dijkstra on it. Verified with 0->1<-2: d(0,2) is now 2 instead of Inf.
| pub fn kamada_kawai_layout<Ty: EdgeType + Sync>( | ||
| py: Python, | ||
| graph: &StablePyGraph<Ty>, | ||
| pos: Option<HashMap<usize, Point>>, |
There was a problem hiding this comment.
Use https://docs.rs/rustworkx-core/latest/rustworkx_core/dictmap/type.DictMap.html, DictMap is closer to Python's dictionary behavior. I am not implying this is required, but we got so many complains about dict order not being deterministic that I just stick to it by default. This comments applies everywhere.
There was a problem hiding this comment.
Swapped pos. Kept fixed as HashSet since the only operations on it are contains and is_empty (matches spring_layout's precedent at src/layout/spring.rs). Happy to also switch fixed if you'd prefer.
|
There’s a CI failure but nothing too hard. |
The dispatcher fallthrough should only raise TypeError - the leftover networkx_converter body was unreachable after the raise and duplicated the real networkx_converter at line 948.
DictMap preserves insertion order matching Python dict semantics. The fixed parameter remains HashSet since order is never observed (only contains/is_empty are called on it).
Replaces hand-rolled edge weight extraction with the codebase helper that already handles weight_fn=None via default_weight, matching the pattern used by bellman-ford, floyd-warshall, and other shortest-path functions. Negative-weight validation stays since the helper doesn't check (and Dijkstra requires non-negative weights).
Replaces the min(d_ij, d_ji) symmetrisation with an explicit undirected graph built from the source topology. The min-based approach failed for weakly-connected digraphs: e.g. graph 0->1<-2 yielded d(0,2)=Inf because neither directed traversal can reach 0 from 2 or vice versa. Building an undirected view gives d(0,2)=2 via the path 0-1-2.
Tests that 0->1<-2->3<-4 (weakly connected, no source-to-sink directed path) gets a sensible layout with d(0,4) > d(0,1). Previously the min-based distance symmetrisation gave d(0,4)=Inf for this topology; the undirected-view fix in the prior commit makes it d(0,4)=4.
Coverage Report for CI Build 26081799484Coverage increased (+0.05%) to 94.722%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
4 participants
Adds
rustworkx.kamada_kawai_layout, supporting bothPyGraphandPyDiGraphinputs. Implements the original Kamada-Kawai (1989) algorithm: an outer loop selects the node with the largest partial-gradient norm and an inner loop applies a 2D Newton step against the local Hessian until convergence.Disconnected graphs are laid out by running Kamada-Kawai independently on each connected component and packing the components in a horizontal row, which avoids the visual collapse seen with single-objective Kamada-Kawai on disconnected inputs. Directed graphs have their distance matrix symmetrised before optimising (Kamada-Kawai is fundamentally undirected).
Related to #438
Example outputs
Side-by-side comparisons against
networkx.kamada_kawai_layout(NetworkX 3.6.1, which uses L-BFGS-B). Both produce K-K minima; differences are rotation/reflection of the same energy minimum.Florentine families (15 nodes, connected)
Medici is centrally placed in both. Topologically equivalent under reflection.
3x4 hexagonal lattice (24 nodes, connected)
Both recover the regular hexagonal grid structure.
Three disconnected components: K4, C5, K3
NetworkX 3.6.1 solves K-K globally on the full distance matrix; components overlap when solved jointly (left). This implementation solves each component independently and packs them horizontally (right).