Skip to content

Fix layer collisions in circuit drawer for nested BoxOp instructions#16568

Open
Prashik123 wants to merge 5 commits into
Qiskit:mainfrom
Prashik123:fix/boxop-layer-collisions
Open

Fix layer collisions in circuit drawer for nested BoxOp instructions#16568
Prashik123 wants to merge 5 commits into
Qiskit:mainfrom
Prashik123:fix/boxop-layer-collisions

Conversation

@Prashik123

@Prashik123 Prashik123 commented Jul 11, 2026

Copy link
Copy Markdown

Description

Hello Team Qiskit! This PR addresses the visual collision bug in the circuit drawer where nested instructions inside block operations (like BoxOp) would incorrectly overlap into a single layer.

What was happening:
When the text drawer (and by extension, the MPL drawer) processed the body of a BoxOp via _get_layered_instructions, it calculated the instruction's span based entirely on the inner virtual qubits (e.g., 0, 1, 2). It didn't account for how those qubits were permuted or mapped to the parent circuit's physical wires, causing overlapping gates to mistakenly share the same visual layer.

How it was fixed:
I updated the layout resolution logic to make it context-aware:

  • Added an optional qubit_mapping parameter to _get_layered_instructions.
  • Passed this mapping down into _LayerSpooler, allowing the insertable and _any_crossover checks to evaluate the true physical span of nested operations based on the outer circuit.
  • Updated add_control_flow in text.py to generate and pass this dictionary mapping when it recursively calls _get_layered_instructions.

Fixes #16510

Use of AI Tools

  • Tool: Gemini (Version: 3.1 Pro Extended)
  • Usage: I used Gemini as a conversational tutor to help analyze the repository architecture (specifically locating the nested layout logic inside _utils.py and text.py) and to troubleshoot local Rust environment compilation issues on Windows. All code changes submitted in this PR are my own original work. I have fully reviewed, tested, and understood the resulting fix, and it complies with the Qiskit license and CLA obligations.

Pull Request Author Checklist

  • The code follows the code style of the project and successfully passes the CI tests (tox -elint / black / ruff).
  • The docstrings have been updated to reflect the new qubit_mapping parameter.
  • I have signed the CLA.
  • The PR has a concise and explanatory title.
  • This PR addresses an open issue and uses the Fixes #16510 syntax to link it.

@Prashik123
Prashik123 requested a review from a team as a code owner July 11, 2026 05:45
@Prashik123
Prashik123 requested a review from Cryoris July 11, 2026 05:45
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jul 11, 2026
@qiskit-bot

Copy link
Copy Markdown
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 following people are relevant to this code:

@CLAassistant

CLAassistant commented Jul 11, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ihincks ihincks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR pipes through a new qubit_mapping argument. But I am wondering:

Why not use the existing wire_map, which I think is exactly the same thing?

I'm also wondering if you checked whether this PR fixes the issue that was posted? Reading the diff, I don't see how it would.

Eventually, this PR will need a changelog and tests.

@ihincks

ihincks commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Reading the diff, I don't see how it would.

I just ran the repro from the linked issue, and it does not appear to fix the issue. Am I missing something?

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community PR PRs from contributors that are not 'members' of the Qiskit repo

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Circuit drawer has layer collisions in boxes

4 participants