Add sxdg, now that it is supported by qiskit #4688
Conversation
| 'sx': QasmGateStatement( | ||
| qasm_gate='sx', num_params=0, num_args=1, cirq_gate=ops.XPowGate(exponent=0.5) | ||
| ), | ||
| 'sxdc': QasmGateStatement( |
| elif self._exponent % 2 == 0.5: | ||
| return args.format('sx {0};\n', qubits[0]) | ||
| elif self._exponent % 2 == 1.5: | ||
| return args.format('sxdg {0};\n', qubits[0]) |
There was a problem hiding this comment.
There is a general question here: to what extent should _qasm_ be simplifying the circuit using known gate properties like X**2=I? On one hand, we could argue that we should preserve programmer's intention in writing something like rx(2.5pi) on the grounds that perhaps they know some low-level hardware details (e.g. gate timing or noise) and simplification would deny them a useful control knob. On the other hand, we could argue for mathematical simplicity and predictability for example to enable the consumer of the QASM to make some reasonable assumptions about the range of parameters such as rotation angles (e.g. they may expect RX rotation angle to be in say [0, 2pi)).
The mathematician in me prefers the latter, but the hacker wants the former. I could live with either. However, the current code bothers me a little bit since by failing to apply simplification for the case of even integer exponents it does not take a uniform stance on the above question.
Suggestions:
- To make the mathematician happy: handle
self._exponent % 2 == 0, e.g. by returning empty string (if this is a valid behavior for_qasm_) or explicit identity gate or at least "rx(0)" for all even integer exponents. - To make the hacker happy: remove
% 2, i.e. preserve the old treatment of exponents.
My preference is for 2, but I leave the choice up to you.
There was a problem hiding this comment.
A variant of option 1 also changes
return args.format('rx({0:half_turns}) {1};\n', self._exponent, qubits[0])to
return args.format('rx({0:half_turns}) {1};\n', self._exponent % 2, qubits[0])There was a problem hiding this comment.
Chose 2. I also chose to make it X**(-0.5) as that feels like the most likely use case.
It feels to me like this is one of those places where we should have had a design where the gate stuff sits on one side and the serialization / instruction set stuff sits on the other side, then there might be default way to use gate stuff to do instruction stuff, but this could be mediated by something that can make other decisions.
There was a problem hiding this comment.
Yeah, I agree it'd be great to have a design clearly stating such things, e.g. adopting or rejecting a principle like
serialization code must be faithful at the parameter level, not merely at the gate algebra level.
As it is, I'm not even sure cirq's behavior is consistent across various gates.
| elif self._exponent % 2 == 0.5: | ||
| return args.format('sx {0};\n', qubits[0]) | ||
| elif self._exponent % 2 == 1.5: | ||
| return args.format('sxdg {0};\n', qubits[0]) |
dabacon
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
Finishes off #3728