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
Fixes digraph_union
if merge_edges
is set to true.
#439
Conversation
Previously, `digraph_union` would falsely keep or delete edges if `merge_edges` is set to true. This commit fixes the logic of `digraph_union` to skip an edge from the second graph if both its endpoints were merged to nodes from the first graph and these nodes already share an edge with equal weight data. At the same time, a new function `graph_union` was added that returns the union of two `PyGraph`s. Closes Qiskit#432.
Pull Request Test Coverage Report for Build 1213842881
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, definitely an improvement in the logic and does make the behavior of merge_edges more clear. Just a few inline comments and suggestions. The other thing is do you think it's worth adding a dispatch function union
to retworkx/__init__.py
?
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the quick updates
Previously,
digraph_union
would falsely keep or deleteedges if
merge_edges
is set to true. This commit fixesthe logic of
digraph_union
to skip an edge from the secondgraph if both its endpoints were merged to nodes from the
first graph and these nodes already share an edge with equal
weight data. At the same time, a new function
graph_union
was added that returns the union of two
PyGraph
s.Closes #432.