Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix implementation of tensor product for undirected graphs #600

Merged
merged 4 commits into from
May 3, 2022

Conversation

derbuihan
Copy link
Contributor

This PR fixes the implementation of tensor product for undirected graphs.

Example:

import networkx as nx

g = nx.path_graph(2)
h = nx.path_graph(2)
r = nx.tensor_product(g, h)

r.edges() # [((0, 0), (1, 1)), ((0, 1), (1, 0))]

However, this calculation in retworkx outputs [((0, 0), (1, 1)), ((1, 1), (0, 0))].
I fixed the tensor product in retworkx to output the same result as networkx.

@coveralls
Copy link

coveralls commented Apr 30, 2022

Pull Request Test Coverage Report for Build 2265616434

  • 18 of 18 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.005%) to 97.194%

Files with Coverage Reduction New Missed Lines %
src/shortest_path/all_pairs_dijkstra.rs 1 98.54%
Totals Coverage Status
Change from base Build 2265302597: -0.005%
Covered Lines: 11294
Relevant Lines: 11620

💛 - Coveralls

Copy link
Collaborator

@georgios-ts georgios-ts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Can you also add a release note describing the bug fix?

src/tensor_product.rs Show resolved Hide resolved
@derbuihan
Copy link
Contributor Author

@georgios-ts
Even though tensor product has not been released yet, is it necessary to add the bug fix to relaesenotes?

Copy link
Collaborator

@georgios-ts georgios-ts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we don't need a release-note. (I got confused and thought we added tensor product in the previous release).
Thanks for the updates!

@mtreinish mtreinish added the automerge Queue a approved PR for merging label May 3, 2022
@CLAassistant
Copy link

CLAassistant commented May 3, 2022

CLA assistant check
All committers have signed the CLA.

@mtreinish mtreinish changed the title fix implementation of tensor product for undirected graphs Fix implementation of tensor product for undirected graphs May 3, 2022
@mergify mergify bot merged commit 4724646 into Qiskit:main May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Queue a approved PR for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants