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 dag visualization for DAGCircuits without registers #7923

Merged
merged 6 commits into from
Apr 14, 2022

Conversation

mtreinish
Copy link
Member

Summary

This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it falls back to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Details and comments

Fixes #7915

This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it fallsback to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Fixes Qiskit#7915
@mtreinish mtreinish added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Apr 12, 2022
@mtreinish mtreinish added this to the 0.20.1 milestone Apr 12, 2022
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@mtreinish mtreinish removed this from the 0.20.1 milestone Apr 12, 2022
@coveralls
Copy link

coveralls commented Apr 12, 2022

Pull Request Test Coverage Report for Build 2167657410

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 21 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.002%) to 83.943%

Files with Coverage Reduction New Missed Lines %
qiskit/visualization/dag_visualization.py 21 52.73%
Totals Coverage Status
Change from base Build 2167216883: -0.002%
Covered Lines: 54211
Relevant Lines: 64581

💛 - Coveralls

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

We should add a bugfix release note, and potentially a minimal test?

qiskit/visualization/dag_visualization.py Outdated Show resolved Hide resolved
This commit changes the default fallback label when no register match is
found to be of the form q_0, where 0 is the qubit index. This was done
to avoid potential confusion with a register named q which might be in
the circuit and not the first element in the qubits list.
@jakelishman jakelishman added this to the 0.20.1 milestone Apr 14, 2022
@mergify mergify bot merged commit 4463ac3 into Qiskit:main Apr 14, 2022
mergify bot pushed a commit that referenced this pull request Apr 14, 2022
* Fix dag visualization for DAGCircuits without registers

This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it fallsback to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Fixes #7915

* Adjust default label from q[0] to q_0

This commit changes the default fallback label when no register match is
found to be of the form q_0, where 0 is the qubit index. This was done
to avoid potential confusion with a register named q which might be in
the circuit and not the first element in the qubits list.

* Add release note

* Add test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 4463ac3)
mergify bot added a commit that referenced this pull request Apr 14, 2022
* Fix dag visualization for DAGCircuits without registers

This commit fixes the handling of generating visualizations for
DAGCircuit objects that have standalone bit objects. Previously, the
dag_drawer() function was assuming every bit in the dag was a member of
a register. In the case of circuits with standalone bits this is not the
case and would result in a KeyError being raised when the register was
attempted to be used to generate a string label for the node or edge
in the visualization. This fixes this by still using registers if
present, but in the absence of a register it fallsback to using it's
absolute index in the dag to generate a similar label. For example, the
4th qubit will have the label "q[4]" in the output visualization.

Fixes #7915

* Adjust default label from q[0] to q_0

This commit changes the default fallback label when no register match is
found to be of the form q_0, where 0 is the qubit index. This was done
to avoid potential confusion with a register named q which might be in
the circuit and not the first element in the qubits list.

* Add release note

* Add test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 4463ac3)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@mtreinish mtreinish deleted the fix-dag-drawer-no-reg branch April 21, 2022 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DAGs imported from networkx cannot be drawn
4 participants