-
Notifications
You must be signed in to change notification settings - Fork 11
Unroll broadcasted operations when producing QASM from SLR #96
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
Unroll broadcasted operations when producing QASM from SLR #96
Conversation
| elif isinstance(q, tuple): | ||
| if len(q) != op.qsize: | ||
| msg = f"Expected size {op.qsize} got size {len(q)}" | ||
| raise Exception(msg) | ||
| qs = ",".join([str(qi) for qi in q]) | ||
| str_list.append(f"{repr_str} {qs};") | ||
|
|
||
| str_list.append(f"{repr_str} {str(q)};") | ||
| else: | ||
| str_list.append(f"{repr_str} {q};") |
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.
It looks to me like that the unconditional final str_list.append(...) was a bug, and should have been inside an else statement. Otherwise this method first acts the gate on every qs in q and then again appends a gate on (the tuple) q.
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.
To me, SWAP is a unitary gate that could possibly be converted to a physical permute if a compiler is smart enough. There is a need to distinguish the two operations as you may what fine control and choose one over the other and do not want to rely on a compiler.
As far as rewriting QASM, the user should no longer be responsible with debugging that. This is the job of the language (e.g., SLR)/compiler. If the underlying base language had support for Permute, then that would make things cleaner; however, SLR needs to metaprogram features that are not supported by the base language which is essentially what higher languages have to do when converting to lower languages. They provide more facilities then the simpler underlying language provides.
That being said. I want to rewrite the Permute object to be a little more friendly to code generation and just stores the info in a nice manner to be handed off to the code generator function.
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.
Upgrading SWAP to a "native gate" of the hardware is perfectly compatible with thinking of it as a unitary gate. Relabeling qubits does implement the desired unitary operation, and it is an advantage of trapped ion platform that it can perform SWAP gates perfectly 🙂! Not all platforms can do this. This is also a capability that can be leveraged by circuit-level compilers (search this paper for "swap mirroring", for example), but doing so requires that the hardware recognize SWAP as a native gate. This essentially "exposes" the virtual SWAP capability to compilers that work on the level of (say) pytket, cirq, or qiskit.
There is also precedent here: having a virtual native SWAP gate is directly analogous to virtual Rz gates on IBM devices. IBM devices accept Rz instructions, but do not actually apply any physical operations any qubits when asked to "apply" an Rz gate (they instead make a software-level update of a rotating frame).
From the perspective of compiler development, the way to get "fine control" and execute a specific sequence of physical operations is to build the circuit you want, and run it in "verbatim" mode to bypass/disable any compiler. Another option is to have compilation flags that toggle various compiler features. There is no control or choice removed by upgrading the SWAP gate to a native virtual operation (and in fact, at the moment you already "rely on a compiler" to convert a SWAP instruction into 3 MS gates).
Final point:
As far as rewriting QASM, the user should no longer be responsible with debugging that.
As a matter of practice, we have definitely resorted to examining QASM to debug PECOS and would appreciate the option to continue doing so 🙂
In any case, what are your thoughts of unrolling broadcast operations in this PR? Some fix for #95 is necessary to use both Permute and (say) Steane.sz in the same SLR (and Permute is used for Steane.t_tel).
ciaranra
left a comment
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 review and test fixes! |
fixes #95
In my view this fix is a bit of a hack around peculiarities of the
Permuteinstruction, which (at least in its current implementation) is fundamentally inconsistent with broadcasting instructions across a qubit register (see comment). The fix in this PR is to just unroll broadcast operations, and make them act on the individual qubits of a register.I'm happy to fix the broken tests if+when the solution in this PR is approved.
If I were to opine a bit: in my view
Permutelooks to be a re-invention of the SWAP gate, and it would be much more favorable to just use SWAPs instead ofPermutes, and in turn have hardware recognize SWAPs as virtual (handled by some below-QASM level of control software, essentially mutating the map from qubit label --> physical qubit/ion). Inserting a single SWAP/Permute into a circuit shouldn't have the effect of (a) modifying all downstream QASM and (b) making visual inspection and debugging of QASM impossible, because the QASM no longer agrees with the SLR it was produced from (asking SLR topecos.qeclib.qubit.X(qreg_a[0])can now produce the QASMx qreg_b[2], for example). And of course broadcasting instructions across a register is a nice and natural capability to have. Having said that, we still needPermutefor now, so hopefully this is an acceptable fix 🙂