Repository: CQCL/pytket-qiskit

Issue:
Incorrect simulation of nested classical controls on the AerBackend
I was trying a fairly simple test of nested classical controls in quantinuum jupyter lab and ran into an issue. 

Consider a setup with 3 qubits and 3 classical registers. On the first qubit we use the H gate and then prepare a Bell state on the other two qubits using a standard circuit controlled by the state of the first qubit. However, when the Bell state preparation is done using an equivalent classically controlled operation (well, not quite equivalent, but the simulation result should be the same), the simulation result of the whole setup is incorrect. Here is the code:

```python
from pytket.circuit import *
from pytket.circuit.display import render_circuit_jupyter as draw
from pytket.extensions.qiskit import AerBackend, AerStateBackend
from pytket.utils import probs_from_counts

U0 = Circuit(2, 2)
U0.H(0)
# ------ 
# The use of CX version below works fine and returns the correct result in the end
# U0.CX(0, 1)
# U0.measure_all()
# ---------
# The classical control version fails
U0.Measure(0, 0)
U0.X(1, condition_bits=[0], condition_value=1)
U0.Measure(1, 1)

g_U0 = CustomGateDef.define("u0", U0, [])

U = Circuit()
q = U.add_q_register("q", 3)
c = U.add_c_register("c", 3)
U.H(0)
U.Measure(0, 0)
U.add_custom_gate(g_U0, [], [1, 2, 1, 2], condition=if_not_bit(c[0]))

backend = AerBackend()
c_U = backend.get_compiled_circuit(U)

handle = backend.process_circuit(c_U, n_shots=1000)
counts = backend.get_result(handle).get_counts()
print(counts)
print(probs_from_counts(counts))
```
which returns 
```
{(0, 0, 1): 0.258, (0, 1, 1): 0.243, (1, 0, 0): 0.499}
```
while it should return 
```
{(0, 0, 0): 0.256, (0, 1, 1): 0.253, (1, 0, 0): 0.491}
```
as in the case of using CX in the custom nested gate.

Discussion:
@CalMacCQ: Hi @dandanua,  Thank you for raising an issue. We'll take a look and get back to you. 

Can I ask which version of pytket/pytket-qiskit you are using?@dandanua: @CalMacCQ 

Hi, it's the one that Quantinuum Nexus is using in their Jupyter Lab, within the standard Python kernel. I don't know how to check the version number from there. @CalMacCQ: Ah thanks, good to know. thats enough for me. I'll dig into the issue soon.

I think you should just be able to do a `!` followed by the shell command in a python notebook cell to find package verions.

```
! pip show pytket-qiskit 
```@dandanua: Version: 0.56.0

BTW, I've tried to run the same circuit against the H1-Emulator using the "getting-started" notebook workflow. The compilation job ran successfully and produced some [transformed circuit](https://imgur.com/a/TOGFBBl) but the execution job has failed with the error 
```
JobError: Job errored with detail: <class 'pytket.backends.backend_exceptions.CircuitNotValidError'>: Circuit with index 0 in submitted does not satisfy GateSetPredicate:{ TK2 ExplicitPredicate CopyBits ClExpr WASM ClassicalExpBox ZZMax ZZPhase Barrier MultiBit PhasedX ExplicitModifier Rz RangePredicate Reset SetBits Measure } (try compiling with backend.get_compiled_circuits first).
```

I'm not sure how to fix this, since I already execute the compiled circuit as in the guide. @CalMacCQ: 
> BTW, I've tried to run the same circuit against the H1-Emulator using the "getting-started" notebook workflow. The compilation job ran successfully and produced some [transformed circuit](https://imgur.com/a/TOGFBBl) but the execution job has failed with the error


hmm strange, I'll make a note to look into that as well as the issue above. The transformed circuit link seems not to work for me.

@dandanua: You must have blocked access to imgur. Here is another [link](https://imath.kiev.ua/~dandan/circuit.png)@cqc-alec: It seems that nested conditionals are not handled correctly by `tk_to_qiskit()`. We use `c_if` to apply conditions, but as stated in the [documentation](https://docs.quantum.ibm.com/api/qiskit/qiskit.circuit.Instruction#c_if):

> This is a setter method, not an additive one. Calling this multiple times will silently override any previously set condition; it does not stack.

It is also deprecated: we should probably switch to using `if_test`.@CalMacCQ: Yeah use of `c_if` is deprecated now. Maybe we can fix after #437 is finished.@dandanua: Thanks. `if_else` could be another alternative. Beware, though, there is a Qiskit bug related to such conditionals too https://github.com/Qiskit/qiskit/issues/12084#issuecomment-2452395919. I've been able to avoid this issue in Qiskit by defining inner circuits (qc0 and qc1) explicitly on a subset of the global quantum register. The remapping doesn't work in the example code there. 

And what do you think about the compilation result in Nexus? The compiled circuit has controlled `Phase(0.5)` on the classical bit, which doesn't make any sense. ![Image](https://github.com/user-attachments/assets/7226c624-ccea-4d5c-b2d4-68468d51f78e)
@CalMacCQ: Thanks @dandanua for the heads up regarding the qiskit issue. 

If I'm not mistaken the way to understand the conditional phase command is "Apply a phase of `e^{i *(0.5)* pi}`globally to the circuit if the condition bit is set to 0". The phase isn't "applied" to `c[0]`. 

There may be a better visualisation for this. I can understand how this might not be very intuitive from the diagram. @cqc-alec: Regarding the error submitting the compiled circuit for the `QuantinuumBackend`: the conditional global phase operation is not the issue -- this will simply be ignored when submitting the circuit -- but there does appear to be an error when rebasing the circuit to the Quantinuum native gate set. The X operation inside the nested conditional is not being rebased.@cqc-alec: See https://github.com/CQCL/tket/issues/1727 for the rebasing issue.@cqc-alec: See #446 for the task of supporting conversion of nested conditionals. With #445 we disallow this for now.

Closing this issue as superceded by #446 and https://github.com/CQCL/tket/issues/1727 .

Solution patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 8e56bfa..057bb55 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -4,6 +4,10 @@
 
 # Changelog
 
+## Unreleased
+
+- Reject circuits containing nested conditionals when converting to qiskit.
+
 ## 0.62.0 (December 2024)
 
 - AerBackend now rejects circuits with too many qubits
diff --git a/pytket/extensions/qiskit/qiskit_convert.py b/pytket/extensions/qiskit/qiskit_convert.py
index 5e0f7b0..112a279 100644
--- a/pytket/extensions/qiskit/qiskit_convert.py
+++ b/pytket/extensions/qiskit/qiskit_convert.py
@@ -47,6 +47,7 @@ from pytket.circuit import (
     Bit,
     CircBox,
     Circuit,
+    Conditional,
     Node,
     Op,
     OpType,
@@ -741,23 +742,27 @@ def append_tk_command_to_qiskit(
         # subsequent conditional will handle it
         return Instruction("", 0, 0, [])
     if optype == OpType.Conditional:
-        if op.op.type == OpType.Phase:  # type: ignore
+        assert isinstance(op, Conditional)
+        if op.op.type == OpType.Conditional:
+            # See https://github.com/CQCL/pytket-qiskit/issues/442
+            raise NotImplementedError("Nested conditional not supported")
+        if op.op.type == OpType.Phase:
             # conditional phase not supported
             return InstructionSet()
         if args[0] in range_preds:
-            assert op.value == 1  # type: ignore
+            assert op.value == 1
             condition_bits, value = range_preds[args[0]]  # type: ignore
             args = condition_bits + args[1:]
             width = len(condition_bits)
         else:
-            width = op.width  # type: ignore
-            value = op.value  # type: ignore
+            width = op.width
+            value = op.value
         regname = args[0].reg_name
         for i, a in enumerate(args[:width]):
             if a.reg_name != regname:
                 raise NotImplementedError("Conditions can only use a single register")
         instruction = append_tk_command_to_qiskit(
-            op.op,  # type: ignore
+            op.op,
             args[width:],
             qcirc,
             qregmap,


Test patch:
diff --git a/tests/qiskit_convert_test.py b/tests/qiskit_convert_test.py
index 254044f..97f149f 100644
--- a/tests/qiskit_convert_test.py
+++ b/tests/qiskit_convert_test.py
@@ -1204,3 +1204,15 @@ def test_range_preds_with_conditionals() -> None:
     assert len(qkc) == 2
     assert len(qkc.qubits) == 1
     assert len(qkc.clbits) == 1
+
+
+def test_nested_conditionals() -> None:
+    # https://github.com/CQCL/pytket-qiskit/issues/442
+    c0 = Circuit(1, 1).X(0, condition_bits=[0], condition_value=1)
+    cbox = CircBox(c0)
+    c = Circuit(1, 2)
+    c.add_circbox(cbox, [Qubit(0), Bit(1)], condition_bits=[Bit(0)], condition_value=1)
+    DecomposeBoxes().apply(c)
+    with pytest.raises(NotImplementedError):
+        # For now we do not support conversion of nested conditionals.
+        _qkc = tk_to_qiskit(c)