Add hexagonal lattice graph generator #277

nahumsa · 2021-03-17T22:05:49Z

Related to #150.

Added new functions for the generator module:

undirected hexagonal lattice graph;
Directed hexagonal lattice graph;
~~undirected complete graph;~~

Currently, the hexagonal generator does not support adding weights, because the algorithm to generate the graph needs to add auxiliary nodes and remove them at the end.

Update: Undirected complete graph was removed because it does the same thing that mesh_graph does and the itertools implementation is slower.

coveralls · 2021-03-17T22:55:18Z

Pull Request Test Coverage Report for Build 831926889

110 of 110 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.05%) to 96.632%

Totals
Change from base Build 821058012:	0.05%
Covered Lines:	7059
Relevant Lines:	7305

💛 - Coveralls

georgios-ts

Just to confirm here that it's indeed intentional: it's a generator for the hexagonal graph and not heavy hex (that has some extra nodes in the edges), right?

If rows=0 or cols=0 should we return an empty graph as networkx does?

src/generators.rs

georgios-ts · 2021-03-23T20:27:15Z

src/generators.rs

+    // Add column edges
+    for i in 0..collen {
+        for j in 0..(rowlen - 1) {
+            if i + 1 < rowlen {


I don't believe this check if i + 1 < rowlen is needed, e.g hexagonal_graph(1, 3) will give wrong result.

Oh, thanks. I forgot that I added rowlen - 1 on the for loop.

nahumsa · 2021-03-23T22:51:51Z

Just to confirm here that it's indeed intentional: it's a generator for the hexagonal graph and not heavy hex (that has some extra nodes in the edges), right?

Yep, I'm currently working on heavy hex.

If rows=0 or cols=0 should we return an empty graph as networkx does?

Yep.

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>

@georgios-ts

Removed if statement ` i + 1 < rowlen` because it would yield wrong results as pointed out by @georgios-ts

Added coverage for creating an empty hexagonal graph

mtreinish

A few quick questions inline. Also do we want to add a directed graph version of this generator (networkx lets you do this, for example: nx.hexagonal_lattice_graph(10, 10, create_using=nx.DiGraph))?

mtreinish · 2021-03-24T11:58:56Z

src/generators.rs

+///
+#[pyfunction(multigraph = true)]
+#[text_signature = "(/, rows=None, cols=None, multigraph=True)"]
+pub fn hexagonal_graph(


Is there a reason to not call this function hexagonal_latice_graph like networkx?

Not really. It can be changed to hexagonal_latice_graph.

mtreinish · 2021-03-24T12:03:51Z

src/generators.rs

+    graph.remove_node(nodes[rowlen - 1]);
+    graph.remove_node(
+        nodes[(collen - 1) * rowlen + (rowlen - 1) * ((collen - 1) % 2)],
+    );


Instead of removing the corner nodes here which will result in a graph with holes in the node indices couldn't we just detect the corner nodes in the edge loop and not add the edge?

Sure. I will do an implementation without removing nodes.

Actually deleting the nodes is required for the implementation. Thus, we have two options:

Documenting that we are deleting nodes and which nodes will be missing;

~~Reindexing all nodes~~.

I will take a look at what will be the overhead of reindexing all nodes.

I ran a benchmark comparing reindexing nodes:

Branches Time

without reindexing 0.115

with reindexing 1.043

Reindexing this graph adds a significant overhead, therefore I will choose to document which nodes that we are deleting.

nahumsa · 2021-03-24T12:59:28Z

A few quick questions inline. Also do we want to add a directed graph version of this generator (networkx lets you do this, for example: nx.hexagonal_lattice_graph(10, 10, create_using=nx.DiGraph))?

I guess so. I will do an implementation for directed hexagonal graph.

… into Expand_Generators

nahumsa · 2021-04-13T01:33:49Z

I added the directed version of the hexagonal lattice graph: directed_hexagonal_lattice_graph, but I'm not sure if the name is too big.

tests/generators/test_hexagonal.py

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

mtreinish

The function code LGTM, just a couple comments on the tests. It would be good if we had the tests validating the output edge list is actually a hexagonal lattice. I think once the tests are updated this is ready to go.

mtreinish · 2021-05-03T20:40:09Z

tests/generators/test_hexagonal.py

+import retworkx
+
+
+class TestHexagonalLatticeGraph(unittest.TestCase):


Could you add some tests that use bidirectional=True for directed_hexagonal_lattice_graph?

mtreinish · 2021-05-03T20:41:31Z

tests/generators/test_hexagonal.py

+    def test_hexagonal_graph_2_2(self):
+        graph = retworkx.generators.hexagonal_lattice_graph(2, 2)
+        self.assertEqual(len(graph), 16)
+        self.assertEqual(len(graph.edges()), 19)


It'd be nice to verify that this is actually a hexagonal lattice so we know moving forward that if we make changes that we're not breaking this. Right now all we know is that there are 16 nodes and 19 edges in the graph, not how they're connected.

Sure, I'll add that.

mtreinish

LGTM! Thanks for updating the tests.

nahumsa added 10 commits March 12, 2021 17:16

Added hexagonal graph without weights as input

a6f8fd2

Added more hexagonal tests

acb2542

removed the weight argument

95e9206

Added itertools to dependencies

60b44a4

Added complete_graph

97138bc

Add release notes complete_graph, hexagonal_graph

818198b

Added complete graph

e8fe00b

Removed MANIFEST

e2b5176

Linting

be7ee62

Linting

8e48a0d

mtreinish requested a review from georgios-ts March 19, 2021 16:19

mtreinish assigned georgios-ts Mar 19, 2021

nahumsa added 2 commits March 19, 2021 16:23

Removed complete graph

1d4256e

Removed complete graph test

fff9141

georgios-ts reviewed Mar 23, 2021

View reviewed changes

nahumsa and others added 4 commits March 23, 2021 19:52

Fixed typo on documentation.

05954bd

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>

Removed option for row and cols

98c9ba8

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>

Removed if statement

77163db

Removed if statement ` i + 1 < rowlen` because it would yield wrong results as pointed out by @georgios-ts

Expanded test coverage

aac84df

Added coverage for creating an empty hexagonal graph

georgios-ts approved these changes Mar 24, 2021

View reviewed changes

mtreinish reviewed Mar 24, 2021

View reviewed changes

mtreinish changed the title ~~Expanding generators~~ Add hexagonal lattice graph generator Apr 2, 2021

nahumsa and others added 5 commits April 8, 2021 15:58

Add documentation for the deleted nodes

246b8a1

fix docs typo

400d668

Merge branch 'master' into Expand_Generators

c506524

change name to hexagonal_lattice_graph

0fda1ac

Merge branch 'Expand_Generators' of https://github.com/nahumsa/retworkx…

c1cc9a6

… into Expand_Generators

add directed_hexagonal_lattice_graph

fc7d5ee

nahumsa added 2 commits April 12, 2021 22:43

lint

73fc3fd

lint

b53ef83

mtreinish reviewed Apr 13, 2021

View reviewed changes

tests/generators/test_hexagonal.py Outdated Show resolved Hide resolved

nahumsa and others added 4 commits April 13, 2021 10:20

Fix test name

c2695fc

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

typo on docs

93a2c87

fix typo docs

81aa949

Merge branch 'main' into Expand_Generators

537de7d

mtreinish reviewed May 3, 2021

View reviewed changes

nahumsa and others added 3 commits May 6, 2021 15:32

add edge_list assertion and bidirectional cases

0583470

Merge branch 'main' into Expand_Generators

47ff9b2

run black fmt

868bde2

mtreinish approved these changes May 14, 2021

View reviewed changes

mtreinish merged commit d13c0b8 into Qiskit:main May 14, 2021

mtreinish mentioned this pull request Jun 25, 2021

retworkx.generators.hexagonal_lattice_graph does not generate consecutive nodes #373

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hexagonal lattice graph generator #277

Add hexagonal lattice graph generator #277

nahumsa commented Mar 17, 2021 •

edited

coveralls commented Mar 17, 2021 •

edited

georgios-ts left a comment

georgios-ts Mar 23, 2021

nahumsa Mar 23, 2021

nahumsa commented Mar 23, 2021

mtreinish left a comment

mtreinish Mar 24, 2021

nahumsa Mar 24, 2021

mtreinish Mar 24, 2021

nahumsa Mar 24, 2021

nahumsa Apr 2, 2021 •

edited

nahumsa Apr 8, 2021

nahumsa commented Mar 24, 2021

nahumsa commented Apr 13, 2021

mtreinish left a comment

mtreinish May 3, 2021

nahumsa May 4, 2021

mtreinish May 3, 2021

nahumsa May 4, 2021

mtreinish left a comment

		import retworkx


		class TestHexagonalLatticeGraph(unittest.TestCase):

Add hexagonal lattice graph generator #277

Add hexagonal lattice graph generator #277

Conversation

nahumsa commented Mar 17, 2021 • edited

coveralls commented Mar 17, 2021 • edited

Pull Request Test Coverage Report for Build 831926889

💛 - Coveralls

georgios-ts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nahumsa commented Mar 23, 2021

mtreinish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nahumsa Apr 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nahumsa commented Mar 24, 2021

nahumsa commented Apr 13, 2021

mtreinish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtreinish left a comment

Choose a reason for hiding this comment

nahumsa commented Mar 17, 2021 •

edited

coveralls commented Mar 17, 2021 •

edited

nahumsa Apr 2, 2021 •

edited