[CQT-59] implement SGMQ notation in the CircuitBuilder#675
[CQT-59] implement SGMQ notation in the CircuitBuilder#675
Conversation
elenbaasc
left a comment
elenbaasc
left a comment
There was a problem hiding this comment.
There are many helper functions that have been added to the CircuitBuilder, that are in my opinion overkill. For two qubit gates and the measure instruction we only allow for operand lists of equal lengths (and no tuple style SGMQ notation).
Try to simplify the (added) code significantly. Also, private helper functions shouldn't have doc-strings.
| def test_sgmq_cnot_from_list(self) -> None: | ||
| builder = CircuitBuilder(4) | ||
| builder.CNOT([0, 3], [1, 3], [2, 3]) | ||
| circuit = builder.to_circuit() | ||
|
|
||
| assert circuit.ir.statements == [CNOT(0, 3), CNOT(1, 3), CNOT(2, 3)] |
There was a problem hiding this comment.
This is unwanted behaviour and should raise an error.
The CNOT instruction should only allow lists for the arguments (and there are only 2 input arguments).
The test should be:
| def test_sgmq_cnot_from_list(self) -> None: | |
| builder = CircuitBuilder(4) | |
| builder.CNOT([0, 3], [1, 3], [2, 3]) | |
| circuit = builder.to_circuit() | |
| assert circuit.ir.statements == [CNOT(0, 3), CNOT(1, 3), CNOT(2, 3)] | |
| def test_sgmq_cnot_from_list(self) -> None: | |
| builder = CircuitBuilder(4) | |
| builder.CNOT([0, 1, 2], [3, 3, 3]) | |
| circuit = builder.to_circuit() | |
| assert circuit.ir.statements == [CNOT(0, 3), CNOT(1, 3), CNOT(2, 3)] |
There was a problem hiding this comment.
I understand the general comment that we do not want a tuple style SGMQ, and will remove this functionality. I thought it would be nice if we wanted to apply a two-qubit gate to multiple qubits, to have a one liner specifying a list of tuples with the control&target qubits for each applied gate.
I have a question to understand your vision a bit better. You are ok with having SGMQ for two qubit gates where there are two list arguments, the first lists specifying control qubits, and the second list target qubits?
And overall you would want to restrict SGMQ notation functionality solely to this format for two qubit gates?
| @staticmethod | ||
| def _is_sgmq_expandable(arg: Any) -> bool: | ||
| """Check if an argument should be expanded for SGMQ notation. | ||
|
|
||
| An argument should be expanded if it is: | ||
| - A list (but not a tuple, which is used for axis arguments) | ||
| - A Register (QubitRegister or BitRegister) that is not being used as an index | ||
|
|
||
| Args: | ||
| arg: The argument to check. | ||
|
|
||
| Returns: | ||
| True if the argument should be expanded, False otherwise. | ||
|
|
||
| """ | ||
| return isinstance(arg, (Register, list)) |
There was a problem hiding this comment.
This is overkill for a simple isinstance statement.
| def test_sgmq_cnot_first_qubit_list(self) -> None: | ||
| builder = CircuitBuilder(4) | ||
| builder.CNOT([0, 1, 2], 3) | ||
| circuit = builder.to_circuit() | ||
|
|
||
| assert circuit.ir.statements == [CNOT(0, 3), CNOT(1, 3), CNOT(2, 3)] |
| builder.add_register(controls) | ||
| builder.add_register(targets) | ||
|
|
||
| builder.CNOT(controls, targets[0]) |
There was a problem hiding this comment.
Not permitted (just like measure, the length of the input arguments should match).
| def test_sgmq_cz_first_qubit_list(self) -> None: | ||
| builder = CircuitBuilder(3) | ||
| builder.CZ([0, 1], 2) | ||
| circuit = builder.to_circuit() | ||
|
|
||
| assert circuit.ir.statements == [CZ(0, 2), CZ(1, 2)] |
| def test_sgmq_two_qubit_gate_with_register_slice(self) -> None: | ||
| builder = CircuitBuilder() | ||
| q0 = QubitRegister(4, "q0") | ||
| builder.add_register(q0) | ||
|
|
||
| builder.CZ(q0[0:2], q0[3]) | ||
| circuit = builder.to_circuit() | ||
|
|
||
| assert circuit.ir.statements == [CZ(0, 3), CZ(1, 3)] |
|
|
||
| ### Added | ||
|
|
||
| - Add Single Gate Multi Qubit (SGMQ) notation |
There was a problem hiding this comment.
| - Add Single Gate Multi Qubit (SGMQ) notation | |
| - Add Single Gate Multi Qubit (SGMQ) notation to the `CircuitBuilder` |
| if not args: | ||
| return [args] | ||
|
|
||
| expandable_indices = [i for i, arg in enumerate(args) if self._is_sgmq_expandable(arg)] |
There was a problem hiding this comment.
I'm not sure what is happening here?
| if self._is_tuple_style_sgmq(args): | ||
| return self._expand_tuple_style_args(args) |
There was a problem hiding this comment.
I don't think we want this to be permitted. We should not allow for tuple style SQMG notation. Input (qu)bit arguments are singular, a list, or a Register.
| def _expand_tuple_style_args(args: tuple[Any, ...]) -> list[tuple[Any, ...]]: | ||
| return [tuple(arg) for arg in args] | ||
|
|
||
| def _expand_measure_args(self, args: tuple[Any, ...], expandable_indices: list[int]) -> list[tuple[Any, ...]]: |
There was a problem hiding this comment.
The measure instruction should not require such a complex and specific method. For both the measure instruction and any two-qubit gate, the first two arguments are always the operands.
2 participants
No description provided.