Skip to content

Commit af5a6ba

Browse files
jpacoldmtreinish
jpacold
and
mtreinish
authored
Hexagonal lattice followup (#1232)
* use subtests for parameter-dependent tests * remove `utils` module from `hexagonal_lattice_graph.rs` * pre-compute the number of edges * split Rust test, add Python node position test * fix factor of two in coordinates calculation * fix offset bug when lattice has an odd number of columns * release note for position bug fixes --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
1 parent ffcd38f commit af5a6ba

File tree

4 files changed

+240
-164
lines changed

4 files changed

+240
-164
lines changed

‎releasenotes/notes/hexagonal-lattice-graph-fixes-1cd4aa88f1591701.yaml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
fixes:
3+
- |
4+
Fixed two bugs in the node position calculation done by the generator functions
5+
:func:`.hexagonal_lattice_graph` and :func:`.directed_hexagonal_lattice_graph` when
6+
``with_positions = True``:
7+
8+
* Corrected a scale factor that made all the hexagons in the lattice irregular
9+
* Corrected an indexing bug that positioned the nodes in the last column of
10+
the lattice incorrectly when ``periodic = False`` and ``cols`` is odd

‎rustworkx-core/src/generators/hexagonal_lattice_graph.rs‎

Lines changed: 159 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -17,178 +17,182 @@ use petgraph::visit::{Data, NodeIndexable};
1717

1818
use super::InvalidInputError;
1919

20-
mod utils {
21-
use super::*;
20+
pub struct HexagonalLatticeBuilder {
21+
rowlen: usize, // Number of nodes in each vertical chain
22+
collen: usize, // Number of vertical chains
23+
num_nodes: usize, // Total number of nodes
24+
num_edges: usize, // Total number of edges
25+
bidirectional: bool,
26+
periodic: bool,
27+
}
2228

23-
pub struct HexagonalLatticeBuilder {
24-
rowlen: usize, // Number of nodes in each vertical chain
25-
collen: usize, // Number of vertical chains
26-
num_nodes: usize, // Total number of nodes
29+
impl HexagonalLatticeBuilder {
30+
pub fn new(
31+
rows: usize,
32+
cols: usize,
2733
bidirectional: bool,
2834
periodic: bool,
29-
}
35+
) -> Result<HexagonalLatticeBuilder, InvalidInputError> {
36+
if periodic && (cols % 2 == 1 || rows < 2 || cols < 2) {
37+
return Err(InvalidInputError {});
38+
}
3039

31-
impl HexagonalLatticeBuilder {
32-
pub fn new(
33-
rows: usize,
34-
cols: usize,
35-
bidirectional: bool,
36-
periodic: bool,
37-
) -> Result<HexagonalLatticeBuilder, InvalidInputError> {
38-
if periodic && (cols % 2 == 1 || rows < 2 || cols < 2) {
39-
return Err(InvalidInputError {});
40-
}
40+
let num_edges_factor = if bidirectional { 2 } else { 1 };
41+
42+
let (rowlen, collen, num_nodes, num_edges) = if periodic {
43+
let r_len = 2 * rows;
44+
(
45+
r_len,
46+
cols,
47+
r_len * cols,
48+
num_edges_factor * 3 * rows * cols,
49+
)
50+
} else {
51+
let r_len = 2 * rows + 2;
52+
(
53+
r_len,
54+
cols + 1,
55+
r_len * (cols + 1) - 2,
56+
num_edges_factor * (3 * rows * cols + 2 * (rows + cols) - 1),
57+
)
58+
};
4159

42-
let (rowlen, collen, num_nodes) = if periodic {
43-
let r_len = 2 * rows;
44-
(r_len, cols, r_len * cols)
45-
} else {
46-
let r_len = 2 * rows + 2;
47-
(r_len, cols + 1, r_len * (cols + 1) - 2)
48-
};
49-
50-
Ok(HexagonalLatticeBuilder {
51-
rowlen,
52-
collen,
53-
num_nodes,
54-
bidirectional,
55-
periodic,
56-
})
57-
}
60+
Ok(HexagonalLatticeBuilder {
61+
rowlen,
62+
collen,
63+
num_nodes,
64+
num_edges,
65+
bidirectional,
66+
periodic,
67+
})
68+
}
5869

59-
pub fn build_with_default_node_weight<G, T, F, H, M>(
60-
self,
61-
mut default_node_weight: F,
62-
default_edge_weight: H,
63-
) -> G
64-
where
65-
G: Build + Create + Data<NodeWeight = T, EdgeWeight = M> + NodeIndexable,
66-
F: FnMut() -> T,
67-
H: FnMut() -> M,
68-
G::NodeId: Eq + Hash,
69-
{
70-
// ToDo: be more precise about the number of edges
71-
let mut graph = G::with_capacity(self.num_nodes, self.num_nodes);
72-
let nodes: Vec<G::NodeId> = (0..self.num_nodes)
73-
.map(|_| graph.add_node(default_node_weight()))
74-
.collect();
75-
self.add_edges(&mut graph, nodes, default_edge_weight);
76-
77-
graph
78-
}
70+
pub fn build_with_default_node_weight<G, T, F, H, M>(
71+
self,
72+
mut default_node_weight: F,
73+
default_edge_weight: H,
74+
) -> G
75+
where
76+
G: Build + Create + Data<NodeWeight = T, EdgeWeight = M> + NodeIndexable,
77+
F: FnMut() -> T,
78+
H: FnMut() -> M,
79+
G::NodeId: Eq + Hash,
80+
{
81+
let mut graph = G::with_capacity(self.num_nodes, self.num_edges);
82+
let nodes: Vec<G::NodeId> = (0..self.num_nodes)
83+
.map(|_| graph.add_node(default_node_weight()))
84+
.collect();
85+
self.add_edges(&mut graph, nodes, default_edge_weight);
7986

80-
pub fn build_with_position_dependent_node_weight<G, T, F, H, M>(
81-
self,
82-
mut node_weight: F,
83-
default_edge_weight: H,
84-
) -> G
85-
where
86-
G: Build + Create + Data<NodeWeight = T, EdgeWeight = M> + NodeIndexable,
87-
F: FnMut(usize, usize) -> T,
88-
H: FnMut() -> M,
89-
G::NodeId: Eq + Hash,
90-
{
91-
// ToDo: be more precise about the number of edges
92-
let mut graph = G::with_capacity(self.num_nodes, self.num_nodes);
93-
94-
let lattice_position = |n| -> (usize, usize) {
95-
if self.periodic {
96-
(n / self.rowlen, n % self.rowlen)
87+
graph
88+
}
89+
90+
pub fn build_with_position_dependent_node_weight<G, T, F, H, M>(
91+
self,
92+
mut node_weight: F,
93+
default_edge_weight: H,
94+
) -> G
95+
where
96+
G: Build + Create + Data<NodeWeight = T, EdgeWeight = M> + NodeIndexable,
97+
F: FnMut(usize, usize) -> T,
98+
H: FnMut() -> M,
99+
G::NodeId: Eq + Hash,
100+
{
101+
let mut graph = G::with_capacity(self.num_nodes, self.num_edges);
102+
103+
let lattice_position = |n| -> (usize, usize) {
104+
if self.periodic {
105+
(n / self.rowlen, n % self.rowlen)
106+
} else {
107+
// In the non-periodic case the first and last vertical
108+
// chains have rowlen - 1 = 2 * rows + 1 nodes. All others
109+
// have rowlen = 2 * rows + 2 nodes.
110+
if n < self.rowlen - 1 {
111+
(0, n)
97112
} else {
98-
// In the non-periodic case the first and last vertical
99-
// chains have rowlen - 1 = 2 * rows + 1 nodes. All others
100-
// have rowlen = 2 * rows + 2 nodes.
101-
if n < self.rowlen - 1 {
102-
(0, n)
113+
let k = n - (self.rowlen - 1);
114+
let u = k / self.rowlen + 1;
115+
let v = k % self.rowlen;
116+
if u == self.collen - 1 && u % 2 == 0 {
117+
(u, v + 1)
103118
} else {
104-
let k = n - (self.rowlen - 1);
105-
let u = k / self.rowlen + 1;
106-
let v = k % self.rowlen;
107-
if u == self.collen - 1 {
108-
(u, v + 1)
109-
} else {
110-
(u, v)
111-
}
119+
(u, v)
112120
}
113121
}
114-
};
122+
}
123+
};
115124

116-
let nodes: Vec<G::NodeId> = (0..self.num_nodes)
117-
.map(lattice_position)
118-
.map(|(u, v)| graph.add_node(node_weight(u, v)))
119-
.collect();
120-
self.add_edges(&mut graph, nodes, default_edge_weight);
125+
let nodes: Vec<G::NodeId> = (0..self.num_nodes)
126+
.map(lattice_position)
127+
.map(|(u, v)| graph.add_node(node_weight(u, v)))
128+
.collect();
129+
self.add_edges(&mut graph, nodes, default_edge_weight);
121130

122-
graph
123-
}
131+
graph
132+
}
124133

125-
fn add_edges<G, H, M>(
126-
&self,
127-
graph: &mut G,
128-
nodes: Vec<G::NodeId>,
129-
mut default_edge_weight: H,
130-
) where
131-
G: Build + NodeIndexable + Data<EdgeWeight = M>,
132-
H: FnMut() -> M,
133-
{
134-
let mut add_edge = |u, v| {
135-
graph.add_edge(nodes[u], nodes[v], default_edge_weight());
136-
if self.bidirectional {
137-
graph.add_edge(nodes[v], nodes[u], default_edge_weight());
138-
}
139-
};
134+
fn add_edges<G, H, M>(&self, graph: &mut G, nodes: Vec<G::NodeId>, mut default_edge_weight: H)
135+
where
136+
G: Build + NodeIndexable + Data<EdgeWeight = M>,
137+
H: FnMut() -> M,
138+
{
139+
let mut add_edge = |u, v| {
140+
graph.add_edge(nodes[u], nodes[v], default_edge_weight());
141+
if self.bidirectional {
142+
graph.add_edge(nodes[v], nodes[u], default_edge_weight());
143+
}
144+
};
140145

141-
if self.periodic {
142-
// Add column edges
143-
for i in 0..self.collen {
144-
let col_start = i * self.rowlen;
145-
for j in col_start..(col_start + self.rowlen - 1) {
146-
add_edge(j, j + 1);
147-
}
148-
add_edge(col_start + self.rowlen - 1, col_start);
149-
}
150-
// Add row edges
151-
for i in 0..self.collen {
152-
let col_start = i * self.rowlen + i % 2;
153-
for j in (col_start..(col_start + self.rowlen)).step_by(2) {
154-
add_edge(j, (j + self.rowlen) % self.num_nodes);
155-
}
156-
}
157-
} else {
158-
// Add column edges
159-
for j in 0..(self.rowlen - 2) {
146+
if self.periodic {
147+
// Add column edges
148+
for i in 0..self.collen {
149+
let col_start = i * self.rowlen;
150+
for j in col_start..(col_start + self.rowlen - 1) {
160151
add_edge(j, j + 1);
161152
}
162-
for i in 1..(self.collen - 1) {
163-
for j in 0..(self.rowlen - 1) {
164-
add_edge(i * self.rowlen + j - 1, i * self.rowlen + j);
165-
}
153+
add_edge(col_start + self.rowlen - 1, col_start);
154+
}
155+
// Add row edges
156+
for i in 0..self.collen {
157+
let col_start = i * self.rowlen + i % 2;
158+
for j in (col_start..(col_start + self.rowlen)).step_by(2) {
159+
add_edge(j, (j + self.rowlen) % self.num_nodes);
166160
}
167-
for j in 0..(self.rowlen - 2) {
168-
add_edge(
169-
(self.collen - 1) * self.rowlen + j - 1,
170-
(self.collen - 1) * self.rowlen + j,
171-
);
161+
}
162+
} else {
163+
// Add column edges
164+
for j in 0..(self.rowlen - 2) {
165+
add_edge(j, j + 1);
166+
}
167+
for i in 1..(self.collen - 1) {
168+
for j in 0..(self.rowlen - 1) {
169+
add_edge(i * self.rowlen + j - 1, i * self.rowlen + j);
172170
}
171+
}
172+
for j in 0..(self.rowlen - 2) {
173+
add_edge(
174+
(self.collen - 1) * self.rowlen + j - 1,
175+
(self.collen - 1) * self.rowlen + j,
176+
);
177+
}
173178

174-
// Add row edges
175-
for j in (0..(self.rowlen - 1)).step_by(2) {
176-
add_edge(j, j + self.rowlen - 1);
177-
}
178-
for i in 1..(self.collen - 2) {
179-
for j in 0..self.rowlen {
180-
if i % 2 == j % 2 {
181-
add_edge(i * self.rowlen + j - 1, (i + 1) * self.rowlen + j - 1);
182-
}
179+
// Add row edges
180+
for j in (0..(self.rowlen - 1)).step_by(2) {
181+
add_edge(j, j + self.rowlen - 1);
182+
}
183+
for i in 1..(self.collen - 2) {
184+
for j in 0..self.rowlen {
185+
if i % 2 == j % 2 {
186+
add_edge(i * self.rowlen + j - 1, (i + 1) * self.rowlen + j - 1);
183187
}
184188
}
185-
if self.collen > 2 {
186-
for j in ((self.collen % 2)..self.rowlen).step_by(2) {
187-
add_edge(
188-
(self.collen - 2) * self.rowlen + j - 1,
189-
(self.collen - 1) * self.rowlen + j - 1 - (self.collen % 2),
190-
);
191-
}
189+
}
190+
if self.collen > 2 {
191+
for j in ((self.collen % 2)..self.rowlen).step_by(2) {
192+
add_edge(
193+
(self.collen - 2) * self.rowlen + j - 1,
194+
(self.collen - 1) * self.rowlen + j - 1 - (self.collen % 2),
195+
);
192196
}
193197
}
194198
}
@@ -272,7 +276,7 @@ where
272276
return Ok(G::with_capacity(0, 0));
273277
}
274278

275-
let builder = utils::HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?;
279+
let builder = HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?;
276280

277281
let graph = builder
278282
.build_with_default_node_weight::<G, T, F, H, M>(default_node_weight, default_edge_weight);
@@ -357,7 +361,7 @@ where
357361
return Ok(G::with_capacity(0, 0));
358362
}
359363

360-
let builder = utils::HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?;
364+
let builder = HexagonalLatticeBuilder::new(rows, cols, bidirectional, periodic)?;
361365

362366
let graph = builder.build_with_position_dependent_node_weight::<G, T, F, H, M>(
363367
node_weight,
@@ -596,7 +600,10 @@ mod tests {
596600
hexagonal_lattice_graph_weighted(2, 2, |u, v| (u, v), || (), false, true).unwrap();
597601
assert_eq!(g_weighted.node_count(), 8);
598602
check_expected_edges_directed(&g_weighted, &expected_edges);
603+
}
599604

605+
#[test]
606+
fn test_hexagonal_lattice_graph_node_weights() {
600607
let g: petgraph::graph::UnGraph<(usize, usize), ()> =
601608
hexagonal_lattice_graph_weighted(2, 2, |u, v| (u, v), || (), false, false).unwrap();
602609
let expected_node_weights = vec![

0 commit comments

Comments
 (0)