Added ladder graph generators and test_ladder #705
Conversation
|
Seungwook Woo seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Overall this is a great looking start. Thanks for working on this!. I left a couple inline comments about the tests where I think some improvements are needed. Also, can you delete the .idea files we shouldn't commit local IDE files to the repo because they're for your local development environment. The branch also looks like it needs to be rebased because of a merge conflict with main..
| self.assertEqual(len(graph.edges()), 58) | ||
|
|
||
| def test_ladder_graph_weights(self): | ||
| graph = rustworkx.generators.ladder_graph(weights=list(range(40))) | ||
| self.assertEqual(len(graph), 40) | ||
| self.assertEqual([x for x in range(40)], graph.nodes()) | ||
| self.assertEqual(len(graph.edges()), 58) |
There was a problem hiding this comment.
For these 2 tests it'd be good to add some assertion on the structure of the edges too. For example, if I built a circuit like:
from rustworkx import rx
graph = rx.PyGraph()
graph.add_nodes_from(range(40))
graph.add_edges_from_no_data([(0, 1)*50)We should try to make the tests so they assert the structure of the created graph is correct and if it were to change by mistake on a future change to the code the test will fail if it's no longer a ladder graph.
Seungwook-Woo
force-pushed
the
add-seungwook
branch
from
53460ab
to
15be344
Compare
There was a problem hiding this comment.
Thanks for writing this. I agree with Matthew that we should write tests about the structure of the graph. A test that could be interesting is:
Take the cartesian product of P2 and Pn and checks that it is isomorphic with the graph you returned
You can see this test for the Generalized Petersen Graph for inspiration
Also, I left a suggestion to reuse the grid graph code. I don't know if it is the best idea to reuse the other code, but it does make the implementation of the ladder graph super short
| if weights.is_none() && num_nodes.is_none() { | ||
| return Err(PyIndexError::new_err( | ||
| "num_nodes and weights list not specified", | ||
| )); | ||
| } else if num_nodes.is_none() && weights.as_ref().unwrap().len() % 2 == 1 { | ||
| return Err(PyIndexError::new_err( | ||
| "length of weights must be even numbers", | ||
| )); | ||
| } | ||
| let node_len = if weights.is_none() { | ||
| num_nodes.unwrap() | ||
| } else { | ||
| weights.as_ref().unwrap().len() / 2 | ||
| }; | ||
|
|
||
| if node_len == 0 { | ||
| return Ok(graph::PyGraph { | ||
| graph: StablePyGraph::<Undirected>::default(), | ||
| node_removed: false, | ||
| multigraph, | ||
| attrs: py.None(), | ||
| }); | ||
| } | ||
| let num_edges = 3 * node_len - 2; | ||
| let mut graph = StablePyGraph::<Undirected>::with_capacity(node_len * 2, num_edges); | ||
| match weights { | ||
| Some(weights) => { | ||
| for weight in weights { | ||
| graph.add_node(weight); | ||
| } | ||
| } | ||
| None => { | ||
| (0..(node_len * 2)).for_each(|_| { | ||
| graph.add_node(py.None()); | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| for rail_a in 0..node_len - 1 { | ||
| graph.add_edge(NodeIndex::new(rail_a), NodeIndex::new(rail_a+1), py.None()); | ||
| let rail_b = rail_a + node_len; | ||
| graph.add_edge(NodeIndex::new(rail_b), NodeIndex::new(rail_b+1), py.None()); | ||
| } | ||
| for rung_a in 0..node_len { | ||
| graph.add_edge(NodeIndex::new(rung_a), NodeIndex::new(rung_a+node_len), py.None()); | ||
| } | ||
|
|
||
| Ok(graph::PyGraph { | ||
| graph, | ||
| node_removed: false, | ||
| multigraph, | ||
| attrs: py.None(), | ||
| }) |
There was a problem hiding this comment.
| if weights.is_none() && num_nodes.is_none() { | |
| return Err(PyIndexError::new_err( | |
| "num_nodes and weights list not specified", | |
| )); | |
| } else if num_nodes.is_none() && weights.as_ref().unwrap().len() % 2 == 1 { | |
| return Err(PyIndexError::new_err( | |
| "length of weights must be even numbers", | |
| )); | |
| } | |
| let node_len = if weights.is_none() { | |
| num_nodes.unwrap() | |
| } else { | |
| weights.as_ref().unwrap().len() / 2 | |
| }; | |
| if node_len == 0 { | |
| return Ok(graph::PyGraph { | |
| graph: StablePyGraph::<Undirected>::default(), | |
| node_removed: false, | |
| multigraph, | |
| attrs: py.None(), | |
| }); | |
| } | |
| let num_edges = 3 * node_len - 2; | |
| let mut graph = StablePyGraph::<Undirected>::with_capacity(node_len * 2, num_edges); | |
| match weights { | |
| Some(weights) => { | |
| for weight in weights { | |
| graph.add_node(weight); | |
| } | |
| } | |
| None => { | |
| (0..(node_len * 2)).for_each(|_| { | |
| graph.add_node(py.None()); | |
| }); | |
| } | |
| }; | |
| for rail_a in 0..node_len - 1 { | |
| graph.add_edge(NodeIndex::new(rail_a), NodeIndex::new(rail_a+1), py.None()); | |
| let rail_b = rail_a + node_len; | |
| graph.add_edge(NodeIndex::new(rail_b), NodeIndex::new(rail_b+1), py.None()); | |
| } | |
| for rung_a in 0..node_len { | |
| graph.add_edge(NodeIndex::new(rung_a), NodeIndex::new(rung_a+node_len), py.None()); | |
| } | |
| Ok(graph::PyGraph { | |
| graph, | |
| node_removed: false, | |
| multigraph, | |
| attrs: py.None(), | |
| }) | |
| grid_graph(py, num_nodes, 2, weights, multigraph) |
The syntax might not be correct, but the ladder graph is equivalent to a n x 2 grid graph hence we probably want to reuse most of the other code
<Ladder graph generator for generators module in #150 >
What?
I've added a ladder graph generator for generators module. For information about ladder graph, see NetworkX's ladder graph function.
Why?
This change can offer more options for quickly creating graphs. See #150 for more information.
How?
A ladder graph of length n is planar, undirected graph with 2n vertices and 3n-2 edges.
The graph consists of two path graphs and each node in a path is connected to a corresponding node in another path.
Edges in a path graph are called rails and edges between paths are called rungs.
For a weighted ladder graph, the function checks the length of weights list which should be even number.
Testing?
I've added a test file for ladder graph generator.
I tested cases for generating 1) ladder graphs, 2) weighted ladder graphs, 3) ladder graphs with no weights or num, and 4) zero length ladder graph.
I tested it 'tox -epy -- -n rustworkx_tests/generators/test_ladder.py' and got successful result.