Skip to content

Commit

Permalink
fix: adjacency matrix for csr and adjacency list (#648)
Browse files Browse the repository at this point in the history
It was found out that the current trait implementations in `CSR`
and `List` weren't working as expected. Problems being fixed:
* `List`: This data structure is directed, but the adjacency matrix was
being built as if it was undirected, adding both `a->b` and `b->a` edges
to the matrix if even if only one of them was present in the graph.
Also, it was using `node_count` to index the adjacency matrix rows, but
was using `edge_count` to query it, resulting in wrong results.
* `CSR`: Was not accounting for undirected graphs
  • Loading branch information
BryanCruz authored Dec 29, 2024
1 parent 9fda6bb commit 7fa3aac
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 6 deletions.
5 changes: 1 addition & 4 deletions src/adj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,15 +641,12 @@ where
for edge in self.edge_references() {
let i = edge.source().index() * n + edge.target().index();
matrix.put(i);

let j = edge.source().index() + n * edge.target().index();
matrix.put(j);
}
matrix
}

fn is_adjacent(&self, matrix: &FixedBitSet, a: NodeIndex<Ix>, b: NodeIndex<Ix>) -> bool {
let n = self.edge_count();
let n = self.node_count();
let index = n * a.index() + b.index();
matrix.contains(index)
}
Expand Down
9 changes: 7 additions & 2 deletions src/csr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,8 +802,13 @@ where
let n = self.node_count();
let mut matrix = FixedBitSet::with_capacity(n * n);
for edge in self.edge_references() {
let index = n * edge.source().index() + edge.target().index();
matrix.put(index);
let i = n * edge.source().index() + edge.target().index();
matrix.put(i);

if !self.is_directed() {
let j = edge.source().index() + n * edge.target().index();
matrix.put(j);
}
}
matrix
}
Expand Down
202 changes: 202 additions & 0 deletions tests/adjacency_matrix.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
use petgraph::{
adj::List,
csr::Csr,
visit::{EdgeRef, GetAdjacencyMatrix, GraphProp, IntoEdgeReferences, IntoNodeIdentifiers},
Directed, EdgeType, Graph, Undirected,
};

#[cfg(feature = "graphmap")]
use petgraph::graphmap::GraphMap;

#[cfg(feature = "matrix_graph")]
use petgraph::matrix_graph::MatrixGraph;

#[cfg(feature = "stable_graph")]
use petgraph::stable_graph::StableGraph;

fn test_adjacency_matrix<G>(g: G)
where
G: GetAdjacencyMatrix + IntoNodeIdentifiers + IntoEdgeReferences + GraphProp,
{
let matrix = g.adjacency_matrix();
let node_ids: Vec<G::NodeId> = g.node_identifiers().collect();
let edges: Vec<(G::NodeId, G::NodeId)> = g
.edge_references()
.map(|edge| (edge.source(), edge.target()))
.collect();

for &a in &node_ids {
for &b in &node_ids {
if edges.contains(&(a, b)) || (!g.is_directed() && edges.contains(&(b, a))) {
assert!(g.is_adjacent(&matrix, a, b));
} else {
assert!(!g.is_adjacent(&matrix, a, b));
}
}
}
}

fn test_adjacency_matrix_for_graph<Ty: EdgeType>() {
for (order, edges) in TEST_CASES {
let mut g: Graph<(), (), Ty, u16> = Graph::with_capacity(order, edges.len());

for _ in 0..order {
g.add_node(());
}
g.extend_with_edges(edges);

test_adjacency_matrix(&g);
}
}

#[test]
fn test_adjacency_matrix_for_graph_directed() {
test_adjacency_matrix_for_graph::<Directed>();
}

#[test]
fn test_adjacency_matrix_for_graph_undirected() {
test_adjacency_matrix_for_graph::<Undirected>();
}

#[cfg(feature = "stable_graph")]
fn test_adjacency_matrix_for_stable_graph<Ty: EdgeType>() {
for (order, edges) in TEST_CASES {
let mut g: StableGraph<(), (), Ty, u16> = StableGraph::with_capacity(order, edges.len());

for _ in 0..order {
g.add_node(());
}
g.extend_with_edges(edges);

test_adjacency_matrix(&g);
}
}

#[cfg(feature = "stable_graph")]
#[test]
fn test_adjacency_matrix_for_stable_graph_directed() {
test_adjacency_matrix_for_stable_graph::<Directed>();
}

#[cfg(feature = "stable_graph")]
#[test]
fn test_adjacency_matrix_for_stable_graph_undirected() {
test_adjacency_matrix_for_stable_graph::<Undirected>();
}

#[cfg(feature = "graphmap")]
fn test_adjacency_matrix_for_graph_map<Ty: EdgeType>() {
for (order, edges) in TEST_CASES {
let mut g: GraphMap<u16, (), Ty> = GraphMap::with_capacity(order, edges.len());

for i in 0..order {
g.add_node(i as u16);
}
for &(a, b) in edges {
g.add_edge(a, b, ());
}

test_adjacency_matrix(&g);
}
}

#[cfg(feature = "graphmap")]
#[test]
fn test_adjacency_matrix_for_graph_map_directed() {
test_adjacency_matrix_for_graph_map::<Directed>();
}

#[cfg(feature = "graphmap")]
#[test]
fn test_adjacency_matrix_for_graph_map_undirected() {
test_adjacency_matrix_for_graph_map::<Undirected>();
}

#[cfg(feature = "matrix_graph")]
fn test_adjacency_matrix_for_matrix_graph<Ty: EdgeType>() {
for (order, edges) in TEST_CASES {
let mut g: MatrixGraph<(), (), Ty> = MatrixGraph::with_capacity(order);

for _ in 0..order {
g.add_node(());
}
g.extend_with_edges(edges);

test_adjacency_matrix(&g);
}
}

#[cfg(feature = "matrix_graph")]
#[test]
fn test_adjacency_matrix_for_matrix_graph_directed() {
test_adjacency_matrix_for_matrix_graph::<Directed>();
}

#[cfg(feature = "matrix_graph")]
#[test]
fn test_adjacency_matrix_for_matrix_graph_undirected() {
test_adjacency_matrix_for_matrix_graph::<Undirected>();
}

fn test_adjacency_matrix_for_csr<Ty: EdgeType>() {
for (order, edges) in TEST_CASES {
let mut g: Csr<(), (), Ty, u16> = Csr::new();

for _ in 0..order {
g.add_node(());
}
for &(a, b) in edges {
g.add_edge(a, b, ());
}

test_adjacency_matrix(&g);
}
}

#[test]
fn test_adjacency_matrix_for_csr_directed() {
test_adjacency_matrix_for_csr::<Directed>();
}

#[test]
fn test_adjacency_matrix_for_csr_undirected() {
test_adjacency_matrix_for_csr::<Undirected>();
}

#[test]
fn test_adjacency_matrix_for_adj_list() {
for (order, edges) in TEST_CASES {
let mut g: List<(), u16> = List::with_capacity(order);

for _ in 0..order {
g.add_node();
}

for &(a, b) in edges {
g.add_edge(a, b, ());
}

test_adjacency_matrix(&g);
}
}

// Test cases format: (graph order, graph edges)
#[rustfmt::skip]
const TEST_CASES: [(usize, &[(u16, u16)]); 10] = [
// Empty Graphs
(0, &[]),
(1, &[]),
(2, &[]),
// Graph with a loop
(2, &[(0, 0)]),
// Small Graphs
(5, &[(0, 2), (0, 4), (1, 3), (3, 4)]),
(6, &[(2, 3)]),
(9, &[(1, 4), (2, 8), (3, 7), (4, 8), (5, 8)]),
// Complete Graphs
(2, &[(0, 1)]),
(7, &[(0, 1), (0, 2), (0, 3), (0, 4), (0, 5), (0, 6), (1, 2), (1, 3), (1, 4), (1, 5), (1, 6), (2, 3), (2, 4), (2, 5), (2, 6), (3, 4), (3, 5), (3, 6), (4, 5), (4, 6), (5, 6)]),
// Petersen
(10, &[(0, 1), (0, 4), (0, 5), (1, 2), (1, 6), (2, 3), (2, 7), (3, 4), (3, 8), (4, 9), (5, 7), (5, 8), (6, 8), (6, 9), (7, 9)]),
];

0 comments on commit 7fa3aac

Please sign in to comment.