feat: handle `IfElseOp` in `qiskit_to_tk` #437

CalMacCQ · 2024-12-17T14:37:58Z

Description

Converting a qiskit IfElseOp as two conditional pytket CircBoxes. This sort of control flow is probably better expressed in the HUGR data structure than as a pytket Circuit/CircBox.

Theres a C.I. failure for one of the tests I added that I can't reproduce locally -> https://github.com/CQCL/pytket-qiskit/actions/runs/12811218709/job/35720030508#step:7:786

Needs more diverse test cases before merging and we also need to decide if we want to handle more complex conditions.

NOTE: For now I've restricted to conditions on a single bit but we should generalise this. This PR is already quite busy.

Looking at the qiskit docs they also have switch-case as well as for and while loops which we may want to support in the future.

https://docs.quantum.ibm.com/guides/classical-feedforward-and-control-flow

Example

# define qiskit program
qubits = QuantumRegister(2)
clbits = ClassicalRegister(2)
circuit = QuantumCircuit(qubits, clbits)
(q0, q1) = qubits
(c0, c1) = clbits

circuit.h(q0)
circuit.measure(q0, c0)

with circuit.if_test((c0, 1)) as else_:
    circuit.h(q1)
with else_:
    circuit.x(q1)
circuit.measure(q1, c1)

Now the corresponding pytket circuit looks like

tkc = qiskit_to_tk(circuit)

The If and Else boxes contain a single $H$ and $X$ gate respectively.

I thought about putting string representing the conditon inside the "If" label however this could be awkward. Hopefully its clear from the diagram whats going on.

This conversion ended up being slighly tricker than I expected because the qiskit circuits which define the true_body and false_body do not have registers. Managed to get around this by using the register from the circuit containing the IfElseOp and cleaning up empty wires with Circuit.remove_blank_wires to ensure the CircBox is of the correct size.

Related issues

closes #415

Checklist

I have performed a self-review of my code.
I have commented hard-to-understand parts of my code.
I have made corresponding changes to the public API documentation.
I have added tests that prove my fix is effective or that my feature works.
I have updated the changelog with any user-facing changes.

pytket/extensions/qiskit/qiskit_convert.py

CalMacCQ · 2024-12-31T13:06:05Z

The test case I added is now working after 52458ab.

I'll need to add more diverse test cases before I'm confident in merging this as well as negate the "else" condition properly and see what the story is for more complex conditions.

tests/qiskit_convert_test.py

CalMacCQ · 2025-01-16T14:44:07Z

pytket/extensions/qiskit/qiskit_convert.py

+
+    # Coniditions must be on a single bit (for now)
+    assert len(if_else_op.condition) == 2
+    if not isinstance(if_else_op.condition[0], Clbit):


As I said in the description I've restricted to the case of single bit conditions for now. However it should be easy enough to generalise for conditons on values of entire registers in a follow up PR.

Should we check that len(bits) == 1, or perhaps change the signature of this function to take a single Bit?

I prefer the former suggestion as the restriction to a single bit should be removable soon

cqc-alec

The PR description says that this closes #366 , but this does not change tk_to_qiskit a all does it? Did you intend to address that direction as well?

cqc-alec · 2025-01-17T08:48:11Z

tests/qiskit_convert_test.py

+    assert tkc.n_gates_of_type(OpType.Conditional) == 2
+    if_cond, else_cond = tuple(tkc.commands_of_type(OpType.Conditional))
+
+    if_circ = if_cond.op.op.get_circuit()


The mypy check is failing. Probably need to assert that op is a Conditional.

Thanks, this was the issue. It doesn't matter anymore as I've redone the test.

cqc-alec · 2025-01-17T08:53:01Z

pytket/extensions/qiskit/qiskit_convert.py

+    circ = circ_builder.circuit()
+
+    # Coniditions must be on a single bit (for now)
+    assert len(if_else_op.condition) == 2


Better to raise an exception rather than assert -- assert is best reserved for catching internal logic errors.

Yeah I considered this. I should add an exception to do things properly.

My assumption was that the object returned from if_else_op.condition would always be of length 2 however this may not be true.

I think for conditions on multiple bits this could be longer.

see cebf4dd

cqc-alec · 2025-01-17T08:57:02Z

pytket/extensions/qiskit/qiskit_convert.py

+
+    # Coniditions must be on a single bit (for now)
+    assert len(if_else_op.condition) == 2
+    if not isinstance(if_else_op.condition[0], Clbit):


Should we check that len(bits) == 1, or perhaps change the signature of this function to take a single Bit?

cqc-alec · 2025-01-17T08:59:24Z

pytket/extensions/qiskit/qiskit_convert.py

+            args=qubits,
+            condition_bits=bits,
+            # TODO: handle conditions over multiple bits/registers?
+            condition_value=not bool(if_else_op.condition[1]),


Is if_else_op.condition[1] an int? Can we check that it is 0 or 1?

Added a check in cebf4dd

cqc-alec · 2025-01-17T09:03:55Z

tests/qiskit_convert_test.py

+
+    expected_circ.Measure(q1_tk[0], c1_tk[0])
+
+    assert tket_circ_if_else == expected_circ


A similar test including an else condition would be good to add.

Turned the two branch test into an explicit equality check in a40732d

CalMacCQ added 2 commits December 17, 2024 14:35

initial attempt at CircBox handling

c424b24

add temporary if-else test

3b9189a

CalMacCQ requested a review from cqc-melf as a code owner December 17, 2024 14:37

CalMacCQ added 4 commits December 17, 2024 14:43

try adding data for conditional subcircuits

692506f

add a proper test

9be4d45

cleanup

80dddbe

add another TODO

e38ca3e

CalMacCQ commented Dec 17, 2024

View reviewed changes

pytket/extensions/qiskit/qiskit_convert.py Show resolved Hide resolved

CalMacCQ marked this pull request as draft December 17, 2024 14:55

CalMacCQ and others added 10 commits December 17, 2024 14:58

formatting

3843234

Merge branch 'main' into support_if-else

69f9e0a

try if else with register from qiskit circuit

09a0ca2

Merge branch 'main' into support_if-else

89ac299

minor cleanup

96d9efd

more progress on circuit building (still gives RuntimeError)

6518628

try to fix kwargs

f87d814

update test

816e2bd

remove import

2ff1b6f

push latest attempt

ea62c0b

yao-cqc reviewed Dec 18, 2024

View reviewed changes

pytket/extensions/qiskit/qiskit_convert.py Outdated Show resolved Hide resolved

CalMacCQ mentioned this pull request Dec 18, 2024

Support QuantumCircuit.if_test in qiskit_to_tk #415

Closed

CalMacCQ added 5 commits December 31, 2024 10:41

naming fixes

73dc681

a little more progress

d65a347

Merge branch 'main' into support_if-else

3281184

refactor: handle IfElseOp separately

52458ab

make circuit builder function private

8e1edce

CalMacCQ changed the title ~~[DRAFT] feat: handle IfElseOp in qiskit_to_tk~~ feat: handle IfElseOp in qiskit_to_tk Dec 31, 2024

CalMacCQ added enhancement New feature or request circuit_conversion Issues and pull requests related to coverting qiskit circuits to pytket and vice versa labels Jan 1, 2025

CalMacCQ added 2 commits January 2, 2025 18:03

Better varible names for testing

ae6cbfe

link to issue

e479c3e

CalMacCQ mentioned this pull request Jan 6, 2025

Incorrect simulation of nested classical controls on the AerBackend #442

Closed

Merge branch 'main' into support_if-else

4c6453a

CalMacCQ mentioned this pull request Jan 15, 2025

deps: update pytket version to 1.38.0 #447

Merged

5 tasks

CalMacCQ added 3 commits January 16, 2025 13:11

Merge branch 'main' into support_if-else

5fab61f

extend test

83b50e7

expand test (annoying failure)

84e7fc1

CalMacCQ commented Jan 16, 2025

View reviewed changes

tests/qiskit_convert_test.py Outdated Show resolved Hide resolved

CalMacCQ added 4 commits January 16, 2025 13:59

fix test inconsistency

dd4a694

add a gate counting test instead (passing)

ba3671d

raise error for register conditions

aadfe5e

add a test

72554e8

CalMacCQ commented Jan 16, 2025

View reviewed changes

CalMacCQ marked this pull request as ready for review January 16, 2025 14:53

CalMacCQ requested a review from cqc-alec January 16, 2025 14:53

cqc-alec requested changes Jan 17, 2025

View reviewed changes

CalMacCQ added 8 commits January 22, 2025 16:05

clean up If-Else circuit building, error handling

cebf4dd

fix a pylint issue

16c3718

Merge branch 'main' into support_if-else

cf2f717

minor fixes

ce035c0

fix mypy issue

1cbf960

fix a typo

dd5ce89

add a validation test for two branch circuit

a40732d

fix import

7bc58d5

CalMacCQ requested a review from cqc-alec January 22, 2025 17:05

explain limitations in changelog

90c68e7

cqc-alec approved these changes Jan 23, 2025

View reviewed changes

cqc-alec merged commit 47e842a into main Jan 23, 2025
4 of 6 checks passed

CalMacCQ mentioned this pull request Jan 23, 2025

Handle register conditions/multiple bits for IfElseOp in qiskit_to_tk #452

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: handle `IfElseOp` in `qiskit_to_tk` #437

feat: handle `IfElseOp` in `qiskit_to_tk` #437

CalMacCQ commented Dec 17, 2024 •

edited

Loading

CalMacCQ commented Dec 31, 2024 •

edited

Loading

CalMacCQ Jan 16, 2025

cqc-alec Jan 17, 2025

CalMacCQ Jan 22, 2025

cqc-alec left a comment

cqc-alec Jan 17, 2025

CalMacCQ Jan 22, 2025

cqc-alec Jan 17, 2025

CalMacCQ Jan 17, 2025

CalMacCQ Jan 22, 2025

cqc-alec Jan 17, 2025

cqc-alec Jan 17, 2025

CalMacCQ Jan 22, 2025

cqc-alec Jan 17, 2025

CalMacCQ Jan 22, 2025


		expected_circ.Measure(q1_tk[0], c1_tk[0])

		assert tket_circ_if_else == expected_circ

feat: handle IfElseOp in qiskit_to_tk #437

feat: handle IfElseOp in qiskit_to_tk #437

Conversation

CalMacCQ commented Dec 17, 2024 • edited Loading

Description

Example

Related issues

Checklist

CalMacCQ commented Dec 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cqc-alec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: handle `IfElseOp` in `qiskit_to_tk` #437

feat: handle `IfElseOp` in `qiskit_to_tk` #437

CalMacCQ commented Dec 17, 2024 •

edited

Loading

CalMacCQ commented Dec 31, 2024 •

edited

Loading