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

Added visualization for qubit permutations for routed circuits #5848

Merged
merged 11 commits into from
Sep 12, 2022

Conversation

ammareltigani
Copy link
Contributor

Adds a function that visualizes the mapping of qubits after each moment in a routed circuit.

@ammareltigani ammareltigani requested review from a team, vtomole and cduck as code owners August 31, 2022 17:40
@CirqBot CirqBot added the size: M 50< lines changed <250 label Aug 31, 2022
@ammareltigani ammareltigani changed the title Improved routed circuit visualization Added visualization for qubit permutations for routed circuits Aug 31, 2022
Copy link
Collaborator

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me overall. This is a slight departure from the usual "print the circuit to see its diagram" flow, but I think it's a good choice so we don't pollute the circuit with these no-op gates.

q1, q2 = op.qubits
qdict[q1], qdict[q2] = qdict[q2], qdict[q1]
ret_circuit.append(m)
ret_circuit.append(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This injects qubit positions after every moment. Is this intended, or should positions only be injected after swaps which affect them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think originally the plan was to inject qubit position info after every swap, not every moment. but now I am wondering which is more readable. Any thoughts on this? I'll soon push a commit with the former.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My preference would be only after swaps, just to keep minimize bloat. There's an argument to be made that every moment ensures there's always a nearby reference for which qubit is which, but that feels less pressing to me (and likely a non-issue for even moderately complex circuits)

Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM % minor nits.

We can merge once the merge conflicts and nits are resolved.

@@ -494,6 +494,7 @@ def all_subclasses(cls):
cirq.Pauli,
# Private gates.
cirq.transformers.analytical_decompositions.two_qubit_to_fsim._BGate,
cirq.transformers.routing.visualize_routed_circuit._SwapPrintGate,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't include an internal class in the global cirq namespace.

Copy link
Contributor Author

@ammareltigani ammareltigani Sep 12, 2022

Choose a reason for hiding this comment

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

It seems like it has to be included under skip_classes in this file or else I get the error
AssertionError: <class 'cirq.transformers.routing.visualize_routed_circuit._SwapPrintGate'> has no json file, please add a json file or add to the list of classes to be skipped if there is a reason this gate should not round trip to a gate via creating an operation.

@tanujkhattar tanujkhattar merged commit 0e62198 into quantumlib:master Sep 12, 2022
tanujkhattar pushed a commit that referenced this pull request Sep 17, 2022
* added circuit visualization code

* made print gate private and added more tests

* removed unused import and added gate to private list

* addressed comments

* added safeguard for wrong use of RoutingSwapTag

* addressed nits

* added return type for

* added _SwapPrintGate again to  and fixed type issue
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…umlib#5848)

* added circuit visualization code

* made print gate private and added more tests

* removed unused import and added gate to private list

* addressed comments

* added safeguard for wrong use of RoutingSwapTag

* addressed nits

* added return type for

* added _SwapPrintGate again to  and fixed type issue
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
…umlib#5848)

* added circuit visualization code

* made print gate private and added more tests

* removed unused import and added gate to private list

* addressed comments

* added safeguard for wrong use of RoutingSwapTag

* addressed nits

* added return type for

* added _SwapPrintGate again to  and fixed type issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants