add get_inverse features #249
Conversation
| @@ -8,6 +8,7 @@ | |||
| from bqskit.ir.gates.constant.clock import ClockGate | |||
| from bqskit.ir.gates.constant.cpi import CPIGate | |||
| from bqskit.ir.gates.constant.cs import CSGate | |||
| from bqskit.ir.gates.constant.csdg import CSDGGate | |||
There was a problem hiding this comment.
We should keep convention and have the dg be lower case. This applies everywhere and to all new gates.
| 'ZGate', | ||
| 'ZZGate', | ||
| 'ZZDGGate', | ||
| ] |
There was a problem hiding this comment.
We also need to add the new gates to the autodoc in bqskit.ir.gates.__init__.py
If you can automate that, that would be awesome!
| from bqskit.qis.unitary.unitarymatrix import UnitaryMatrix | ||
|
|
||
|
|
||
| class CSDGGate(ConstantGate, QubitGate): |
| """This module implements the CSDGGate.""" | ||
| from __future__ import annotations | ||
|
|
||
| from bqskit.ir.gate import Gate |
There was a problem hiding this comment.
This import should be under a TYPE_CHECKING clause everywhere unless necessary for code execution
| from bqskit.qis.permutation import PermutationMatrix | ||
| from bqskit.utils.typing import is_integer | ||
|
|
||
|
|
||
| class SwapGate(ConstantGate): | ||
| class SwapGate(ConstantGate, QubitGate): |
There was a problem hiding this comment.
The swap gate is only qubit by default. You can do SwapGate(3) for a qutrit swap, please revert.
| radixes: Sequence[int] = [], | ||
| self, | ||
| utry: UnitaryLike, | ||
| radixes: Sequence[int] = [], |
There was a problem hiding this comment.
Please revert formatting, there's a few spots where this comes up
| def get_inverse(self) -> Gate: | ||
| """Return the inverse of this gate.""" | ||
| from bqskit.ir.gates.constant.xxdg import XXDGGate | ||
| return XXDGGate() |
There was a problem hiding this comment.
We should remove XXDG, YYDG, and ZZDG, and just have the inverses of these go to RXX, RYY, and RZZ with the correct inverted angle.
| supposed_to_be_iden = ( | ||
| inv_gate.get_unitary() @ gate.get_unitary() | ||
| ) | ||
| dist = supposed_to_be_iden.get_distance_from(iden, 1) |
There was a problem hiding this comment.
Is get_distance_from the correct way to test here? This will ignore global phase, I have to think we should allow this. If you make the test more strict and just check the norm to identity, for example, do you have any issues?
Also, we should test multiplication both ways, A@B=I and B@A=I.
| iden = np.identity(gate.dim) | ||
| supposed_to_be_iden = (inv_op.get_unitary() @ op.get_unitary()) | ||
| dist = supposed_to_be_iden.get_distance_from(iden, 1) | ||
| assert dist < 1e-10 |
There was a problem hiding this comment.
Can we also do another test for the qasm definitions you added? I think we wrote that test before we even had qasm fully working, so it just checks that its a string. We should ensure that a circuit containing these gates (one at a time) can be saved to and loaded from qasm, while also maintaining the same unitary definition before and after.
fixes Add missing
get_inversedefinitions #227