Support for QuantumRegister in gates supporing qubits #369
Conversation
|
Can you revise the test that is failing in Appveyor? It seems the order of the parameters for |
| If args is None, applies to all the qbits. | ||
| If args is a QuantumRegister, applies to all the qbits in that register. | ||
| If args is a list of QuantumRegister, applies to all of them.""" | ||
|
|
There was a problem hiding this comment.
It looks like args can be a list of QuantumRegister and/or qubit, and the barrier will be applied to all arguments. If that's right, could you add that to the comment?
There was a problem hiding this comment.
Clarified in d3a9d32. Thanks!
qiskit/extensions/standard/swap.py
Outdated
| if isinstance(tgt, QuantumRegister): | ||
| instructions = InstructionSet() | ||
| for j in range(tgt.size): | ||
| instructions.add(self.swap(ctl, (tgt, j))) |
There was a problem hiding this comment.
The swap gate should not allow ctrl and tgt of different sizes I think.
I think swaps between 2 individual qubits or between 2 registers of the same size makes sense. But when the size of registers differ, an exception should be raised.
@awcross1 do you agree?
There was a problem hiding this comment.
@ajavadia Yes, I missed this. I agree that swap should not allow different sized registers as input. Good catch.
There was a problem hiding this comment.
Generally we should be careful when G_{i,j} and G_{i,k} do not commute. Does this happen anywhere else in the PR?
There was a problem hiding this comment.
fixed in 42f6b5a. The valid and invalid cases are listed in the tests. I'm not fully sure what you mean with "G_{i,j} and G_{i,k} cases".
There was a problem hiding this comment.
Thanks @1ucian0 the issue doesn't occur anywhere else. I meant G_{i,j} to denote a two-qubit gate acting on qubits i and j.
| @@ -52,182 +52,424 @@ | |||
| from .common import QiskitTestCase | |||
|
|
|||
|
|
|||
| class TestStandard(QiskitTestCase): | |||
| """Standard Extension Test.""" | |||
| class StandardExtensionTest(QiskitTestCase): | |||
There was a problem hiding this comment.
Would it be possible to add human-readable docstrings to the assertions methods in this class, ideally describing the parameters as well?
|
Thanks @1ucian0 ! |
* no name in the registers means no name :-/
* slip assertResult
* simpler
* 352 fix, test_rz_reg and test_s_reg
* test_sdg_reg
* swap behaves as CX when called with registers as args.
* test_swap_reg_reg_inv
* useless =
* cx
* cx_base
* test_rz_reg_inv
* ry and rx
* test_s_reg_inv
* cy and cz
* barrier
* keeping previous behaivor
* ch
* cu3
* cu1
* crz
* crz and h
* iden
* sdg
* t
* tdg
* u1
* u2
* u3
* ubase
* x
* y
* z
* code organization
* barrier for a list of qbits (and formating)
* adding backwards compatibility to barrier
* format
* ccx and cswap
* format
* this extension of instructionset is not needed
* format
* ordered get_{c,q}regs
* invalid tests
* -from qiskit import InstructionSet
* clarification in barrier usage
* do not allow, swap q,r[1]
* qasm stmt should be in new lines
* docstring
This PR adds support to gates which support qubits to also accept full register, as the spec defines.
Description
The gates that take one or more qubit as args, should also support the full
QuantumRegisteras argument. For example:circuit.h(self.qr). The PR also extends the tests for the added code (and more).Motivation and Context
Trying to write a test for #352 (and #362) I ended up falling the rabbit hole of supporting
QuantumRegisters as arguments in the gates that operate over qubits. I noticed that many of the gates had this feature broken.The most notable cases are:
barrierare extended, in order to behave more similarly to other gates. In addition to the previous*list, now QuantumRegister are supported, also in combination with qubits. Also, if None is passed, it applied the barrier to every QuantumRegister of the circuit (this was a TODO in the code).cxwhen is called with register and bits (Fig 2, page 4). This semantic is now supported.cx_baseandswapare analogue tocx.ccxwhen called with registers. Therefore, I only included the intuitive case in which all the arguments are registers (with the same length):ccx q,r,s=>ccx q[0],r[0],s[0];ccx q[1],r[1],s[1];when length is 2.cswapcase is analogue toccx.BTW, this PR closes #352.
How Has This Been Tested?
make testThe added tests extend the coverage with respect the existing tests:
None of the changes brake backward compatibility.
Types of changes