diff --git a/releasenotes/notes/hexagonal-lattice-graph-fixes-1cd4aa88f1591701.yaml b/releasenotes/notes/hexagonal-lattice-graph-fixes-1cd4aa88f1591701.yaml new file mode 100644 index 000000000..3565e1bb0 --- /dev/null +++ b/releasenotes/notes/hexagonal-lattice-graph-fixes-1cd4aa88f1591701.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixed two bugs in the node position calculation done by the generator functions + :func:`.hexagonal_lattice_graph` and :func:`.directed_hexagonal_lattice_graph` when + ``with_positions = True``: + + * Corrected a scale factor that made all the hexagons in the lattice irregular + * Corrected an indexing bug that positioned the nodes in the last column of + the lattice incorrectly when ``periodic = False`` and ``cols`` is odd diff --git a/rustworkx-core/src/generators/hexagonal_lattice_graph.rs b/rustworkx-core/src/generators/hexagonal_lattice_graph.rs index 0f6571e38..b1b970acf 100644 --- a/rustworkx-core/src/generators/hexagonal_lattice_graph.rs +++ b/rustworkx-core/src/generators/hexagonal_lattice_graph.rs @@ -17,178 +17,182 @@ use petgraph::visit::{Data, NodeIndexable}; use super::InvalidInputError; -mod utils { - use super::*; +pub struct HexagonalLatticeBuilder { + rowlen: usize, // Number of nodes in each vertical chain + collen: usize, // Number of vertical chains + num_nodes: usize, // Total number of nodes + num_edges: usize, // Total number of edges + bidirectional: bool, + periodic: bool, +} - pub struct HexagonalLatticeBuilder { - rowlen: usize, // Number of nodes in each vertical chain - collen: usize, // Number of vertical chains - num_nodes: usize, // Total number of nodes +impl HexagonalLatticeBuilder { + pub fn new( + rows: usize, + cols: usize, bidirectional: bool, periodic: bool, - } + ) -> Result { + if periodic && (cols % 2 == 1 || rows < 2 || cols < 2) { + return Err(InvalidInputError {}); + } - impl HexagonalLatticeBuilder { - pub fn new( - rows: usize, - cols: usize, - bidirectional: bool, - periodic: bool, - ) -> Result { - if periodic && (cols % 2 == 1 || rows < 2 || cols < 2) { - return Err(InvalidInputError {}); - } + let num_edges_factor = if bidirectional { 2 } else { 1 }; + + let (rowlen, collen, num_nodes, num_edges) = if periodic { + let r_len = 2 * rows; + ( + r_len, + cols, + r_len * cols, + num_edges_factor * 3 * rows * cols, + ) + } else { + let r_len = 2 * rows + 2; + ( + r_len, + cols + 1, + r_len * (cols + 1) - 2, + num_edges_factor * (3 * rows * cols + 2 * (rows + cols) - 1), + ) + }; - let (rowlen, collen, num_nodes) = if periodic { - let r_len = 2 * rows; - (r_len, cols, r_len * cols) - } else { - let r_len = 2 * rows + 2; - (r_len, cols + 1, r_len * (cols + 1) - 2) - }; - - Ok(HexagonalLatticeBuilder { - rowlen, - collen, - num_nodes, - bidirectional, - periodic, - }) - } + Ok(HexagonalLatticeBuilder { + rowlen, + collen, + num_nodes, + num_edges, + bidirectional, + periodic, + }) + } - pub fn build_with_default_node_weight( - self, - mut default_node_weight: F, - default_edge_weight: H, - ) -> G - where - G: Build + Create + Data + NodeIndexable, - F: FnMut() -> T, - H: FnMut() -> M, - G::NodeId: Eq + Hash, - { - // ToDo: be more precise about the number of edges - let mut graph = G::with_capacity(self.num_nodes, self.num_nodes); - let nodes: Vec = (0..self.num_nodes) - .map(|_| graph.add_node(default_node_weight())) - .collect(); - self.add_edges(&mut graph, nodes, default_edge_weight); - - graph - } + pub fn build_with_default_node_weight( + self, + mut default_node_weight: F, + default_edge_weight: H, + ) -> G + where + G: Build + Create + Data + NodeIndexable, + F: FnMut() -> T, + H: FnMut() -> M, + G::NodeId: Eq + Hash, + { + let mut graph = G::with_capacity(self.num_nodes, self.num_edges); + let nodes: Vec = (0..self.num_nodes) + .map(|_| graph.add_node(default_node_weight())) + .collect(); + self.add_edges(&mut graph, nodes, default_edge_weight); - pub fn build_with_position_dependent_node_weight( - self, - mut node_weight: F, - default_edge_weight: H, - ) -> G - where - G: Build + Create + Data + NodeIndexable, - F: FnMut(usize, usize) -> T, - H: FnMut() -> M, - G::NodeId: Eq + Hash, - { - // ToDo: be more precise about the number of edges - let mut graph = G::with_capacity(self.num_nodes, self.num_nodes); - - let lattice_position = |n| -> (usize, usize) { - if self.periodic { - (n / self.rowlen, n % self.rowlen) + graph + } + + pub fn build_with_position_dependent_node_weight( + self, + mut node_weight: F, + default_edge_weight: H, + ) -> G + where + G: Build + Create + Data + NodeIndexable, + F: FnMut(usize, usize) -> T, + H: FnMut() -> M, + G::NodeId: Eq + Hash, + { + let mut graph = G::with_capacity(self.num_nodes, self.num_edges); + + let lattice_position = |n| -> (usize, usize) { + if self.periodic { + (n / self.rowlen, n % self.rowlen) + } else { + // In the non-periodic case the first and last vertical + // chains have rowlen - 1 = 2 * rows + 1 nodes. All others + // have rowlen = 2 * rows + 2 nodes. + if n < self.rowlen - 1 { + (0, n) } else { - // In the non-periodic case the first and last vertical - // chains have rowlen - 1 = 2 * rows + 1 nodes. All others - // have rowlen = 2 * rows + 2 nodes. - if n < self.rowlen - 1 { - (0, n) + let k = n - (self.rowlen - 1); + let u = k / self.rowlen + 1; + let v = k % self.rowlen; + if u == self.collen - 1 && u % 2 == 0 { + (u, v + 1) } else { - let k = n - (self.rowlen - 1); - let u = k / self.rowlen + 1; - let v = k % self.rowlen; - if u == self.collen - 1 { - (u, v + 1) - } else { - (u, v) - } + (u, v) } } - }; + } + }; - let nodes: Vec = (0..self.num_nodes) - .map(lattice_position) - .map(|(u, v)| graph.add_node(node_weight(u, v))) - .collect(); - self.add_edges(&mut graph, nodes, default_edge_weight); + let nodes: Vec = (0..self.num_nodes) + .map(lattice_position) + .map(|(u, v)| graph.add_node(node_weight(u, v))) + .collect(); + self.add_edges(&mut graph, nodes, default_edge_weight); - graph - } + graph + } - fn add_edges( - &self, - graph: &mut G, - nodes: Vec, - mut default_edge_weight: H, - ) where - G: Build + NodeIndexable + Data, - H: FnMut() -> M, - { - let mut add_edge = |u, v| { - graph.add_edge(nodes[u], nodes[v], default_edge_weight()); - if self.bidirectional { - graph.add_edge(nodes[v], nodes[u], default_edge_weight()); - } - }; + fn add_edges(&self, graph: &mut G, nodes: Vec, mut default_edge_weight: H) + where + G: Build + NodeIndexable + Data, + H: FnMut() -> M, + { + let mut add_edge = |u, v| { + graph.add_edge(nodes[u], nodes[v], default_edge_weight()); + if self.bidirectional { + graph.add_edge(nodes[v], nodes[u], default_edge_weight()); + } + }; - if self.periodic { - // Add column edges - for i in 0..self.collen { - let col_start = i * self.rowlen; - for j in col_start..(col_start + self.rowlen - 1) { - add_edge(j, j + 1); - } - add_edge(col_start + self.rowlen - 1, col_start); - } - // Add row edges - for i in 0..self.collen { - let col_start = i * self.rowlen + i % 2; - for j in (col_start..(col_start + self.rowlen)).step_by(2) { - add_edge(j, (j + self.rowlen) % self.num_nodes); - } - } - } else { - // Add column edges - for j in 0..(self.rowlen - 2) { + if self.periodic { + // Add column edges + for i in 0..self.collen { + let col_start = i * self.rowlen; + for j in col_start..(col_start + self.rowlen - 1) { add_edge(j, j + 1); } - for i in 1..(self.collen - 1) { - for j in 0..(self.rowlen - 1) { - add_edge(i * self.rowlen + j - 1, i * self.rowlen + j); - } + add_edge(col_start + self.rowlen - 1, col_start); + } + // Add row edges + for i in 0..self.collen { + let col_start = i * self.rowlen + i % 2; + for j in (col_start..(col_start + self.rowlen)).step_by(2) { + add_edge(j, (j + self.rowlen) % self.num_nodes); } - for j in 0..(self.rowlen - 2) { - add_edge( - (self.collen - 1) * self.rowlen + j - 1, - (self.collen - 1) * self.rowlen + j, - ); + } + } else { + // Add column edges + for j in 0..(self.rowlen - 2) { + add_edge(j, j + 1); + } + for i in 1..(self.collen - 1) { + for j in 0..(self.rowlen - 1) { + add_edge(i * self.rowlen + j - 1, i * self.rowlen + j); } + } + for j in 0..(self.rowlen - 2) { + add_edge( + (self.collen - 1) * self.rowlen + j - 1, + (self.collen - 1) * self.rowlen + j, + ); + } - // Add row edges - for j in (0..(self.rowlen - 1)).step_by(2) { - add_edge(j, j + self.rowlen - 1); - } - for i in 1..(self.collen - 2) { - for j in 0..self.rowlen { - if i % 2 == j % 2 { - add_edge(i * self.rowlen + j - 1, (i + 1) * self.rowlen + j - 1); - } + // Add row edges + for j in (0..(self.rowlen - 1)).step_by(2) { + add_edge(j, j + self.rowlen - 1); + } + for i in 1..(self.collen - 2) { + for j in 0..self.rowlen { + if i % 2 == j % 2 { + add_edge(i * self.rowlen + j - 1, (i + 1) * self.rowlen + j - 1); } } - if self.collen > 2 { - for j in ((self.collen % 2)..self.rowlen).step_by(2) { - add_edge( - (self.collen - 2) * self.rowlen + j - 1, - (self.collen - 1) * self.rowlen + j - 1 - (self.collen % 2), - ); - } + } + if self.collen > 2 { + for j in ((self.collen % 2)..self.rowlen).step_by(2) { + add_edge( + (self.collen - 2) * self.rowlen + j - 1, + (self.collen - 1) * self.rowlen + j - 1 - (self.collen % 2), + ); } } } @@ -272,7 +276,7 @@ where return Ok(G::with_capacity(0, 0)); } - let builder = utils::HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?; + let builder = HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?; let graph = builder .build_with_default_node_weight::(default_node_weight, default_edge_weight); @@ -357,7 +361,7 @@ where return Ok(G::with_capacity(0, 0)); } - let builder = utils::HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?; + let builder = HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?; let graph = builder.build_with_position_dependent_node_weight::( node_weight, @@ -596,7 +600,10 @@ mod tests { hexagonal_lattice_graph_weighted(2, 2, |u, v| (u, v), || (), false, true).unwrap(); assert_eq!(g_weighted.node_count(), 8); check_expected_edges_directed(&g_weighted, &expected_edges); + } + #[test] + fn test_hexagonal_lattice_graph_node_weights() { let g: petgraph::graph::UnGraph<(usize, usize), ()> = hexagonal_lattice_graph_weighted(2, 2, |u, v| (u, v), || (), false, false).unwrap(); let expected_node_weights = vec![ diff --git a/src/generators.rs b/src/generators.rs index 7d41a10b4..ac8088d12 100644 --- a/src/generators.rs +++ b/src/generators.rs @@ -1105,8 +1105,8 @@ pub fn full_rary_tree( fn _hexagonal_lattice_node_position(u: usize, v: usize) -> (f64, f64) { let [i, j, a, b, c] = [u, v, u / 2, v % 2, u % 2].map(|val| val as f64); - const SQRT3: f64 = 1.732_050_807_568_877_2_f64; - (0.5 + i + a + b * (c - 0.5), SQRT3 * j) + const HALFSQRT3: f64 = 0.866_025_403_784_438_6_f64; + (0.5 + i + a + b * (c - 0.5), HALFSQRT3 * j) } /// Generate an undirected hexagonal lattice graph. @@ -1122,7 +1122,7 @@ fn _hexagonal_lattice_node_position(u: usize, v: usize) -> (f64, f64) { /// ``rows > 1``, and ``cols > 1``. /// :param bool with_positions: When set to ``True`` each node will be assigned /// a pair of coordinates ``(x, y)`` as a weight. This embeds the nodes in -/// the plane so that each hexagon is regular (with side length 2). +/// the plane so that each hexagon is regular (with side length 1). /// /// :returns: The generated hexagonal lattice graph. /// @@ -1198,7 +1198,7 @@ pub fn hexagonal_lattice_graph( /// ``rows > 1``, and ``cols > 1``. /// :param bool with_positions: When set to ``True`` each node will be assigned /// a pair of coordinates ``(x, y)`` as a payload. This embeds the nodes in -/// the plane so that each hexagon is regular (with side length 2). +/// the plane so that each hexagon is regular (with side length 1). /// /// :returns: The generated directed hexagonal lattice graph. /// diff --git a/tests/generators/test_hexagonal.py b/tests/generators/test_hexagonal.py index 6e09bb8c7..dfdc5a084 100644 --- a/tests/generators/test_hexagonal.py +++ b/tests/generators/test_hexagonal.py @@ -13,6 +13,9 @@ import unittest import rustworkx import networkx +import numpy as np + +import rustworkx.generators class TestHexagonalLatticeGraph(unittest.TestCase): @@ -438,9 +441,12 @@ def test_hexagonal_graph_periodic_degree(self): degree 3 (idea copied from the networkx test suite).""" for nRows in range(2, 8): for nCols in range(2, 8, 2): - graph = rustworkx.generators.hexagonal_lattice_graph(nRows, nCols, periodic=True) - for n in range(graph.num_nodes()): - self.assertEqual(graph.degree(n), 3) + with self.subTest(nRows=nRows, nCols=nCols): + graph = rustworkx.generators.hexagonal_lattice_graph( + nRows, nCols, periodic=True + ) + for n in range(graph.num_nodes()): + self.assertEqual(graph.degree(n), 3) def test_hexagonal_graph_periodic_3_4(self): graph = rustworkx.generators.hexagonal_lattice_graph(3, 4, periodic=True) @@ -564,14 +570,67 @@ def test_hexagonal_graph_periodic_networkx_equivalent(self): networkx are isomorphic for a few cases.""" for nRows in range(2, 8): for nCols in range(2, 8, 2): - graph = rustworkx.generators.hexagonal_lattice_graph(nRows, nCols, periodic=True) + with self.subTest(nRows=nRows, nCols=nCols): + graph = rustworkx.generators.hexagonal_lattice_graph( + nRows, nCols, periodic=True + ) - nx_graph = rustworkx.networkx_converter( - networkx.hexagonal_lattice_graph(nRows, nCols, periodic=True) - ) + nx_graph = rustworkx.networkx_converter( + networkx.hexagonal_lattice_graph(nRows, nCols, periodic=True) + ) - self.assertTrue(rustworkx.is_isomorphic(graph, nx_graph)) + self.assertTrue(rustworkx.is_isomorphic(graph, nx_graph)) def test_hexagonal_graph_periodic_odd_columns(self): with self.assertRaises(ValueError): rustworkx.generators.hexagonal_lattice_graph(4, 5, periodic=True) + + def test_hexagonal_graph_with_positions(self): + graph = rustworkx.generators.hexagonal_lattice_graph(2, 2, with_positions=True) + positions = graph.nodes() + hexagons = [ + [0, 1, 2, 7, 6, 5], + [2, 3, 4, 9, 8, 7], + [6, 7, 8, 13, 12, 11], + [8, 9, 10, 15, 14, 13], + ] + C6 = rustworkx.generators.cycle_graph(6) + for h in hexagons: + self.assertTrue(rustworkx.is_isomorphic(graph.subgraph(h), C6)) + coordinates = np.array([positions[node] for node in h]) + vectors = [coordinates[(ii + 1) % 6] - coordinates[ii] for ii in range(6)] + + for v in vectors: + # Check that each side of the hexagon has length 1 + self.assertAlmostEqual(np.linalg.norm(v), 1.0, 12) + for ii in range(6): + # Check that the angle between each consecutive pair of sides is pi/3 + self.assertAlmostEqual( + np.dot(vectors[ii], vectors[(ii + 1) % 6]), np.cos(np.pi / 3), 12 + ) + + def test_hexagonal_graph_with_positions_odd_number_of_columns(self): + graph = rustworkx.generators.hexagonal_lattice_graph(2, 3, with_positions=True) + positions = graph.nodes() + hexagons = [ + [0, 1, 2, 7, 6, 5], + [2, 3, 4, 9, 8, 7], + [6, 7, 8, 14, 13, 12], + [8, 9, 10, 16, 15, 14], + [11, 12, 13, 19, 18, 17], + [13, 14, 15, 21, 20, 19], + ] + C6 = rustworkx.generators.cycle_graph(6) + for h in hexagons: + self.assertTrue(rustworkx.is_isomorphic(graph.subgraph(h), C6)) + coordinates = np.array([positions[node] for node in h]) + vectors = [coordinates[(ii + 1) % 6] - coordinates[ii] for ii in range(6)] + + for v in vectors: + # Check that each side of the hexagon has length 1 + self.assertAlmostEqual(np.linalg.norm(v), 1.0, 12) + for ii in range(6): + # Check that the angle between each consecutive pair of sides is pi/3 + self.assertAlmostEqual( + np.dot(vectors[ii], vectors[(ii + 1) % 6]), np.cos(np.pi / 3), 12 + )