feat: Improved handling of controlled gates in converters #391

CalMacCQ · 2024-09-12T15:35:14Z

Description

This PR adds support for handling the control_state of a controlled operation in the qiskit_to_tk and tk_to_qiskit converters. The control state is now handled directly instead of adding X gates on the control qubits.

I've also taken the liberty of refactoring out the "QControlBox building" from the CircuitBuildier.add_qiskit_data method in the same spirit as #382

I've also reworked some internals of how unitary gates are converted to and fro. Handling of controlled unitary boxes was added in #372 . ~~I think it'd be fairly easy to add the other direction into this PR too (see #378)~~ (I think its best to do this separately).

Original PR adding control state handling with X gates -> #118

Related issues

#378
#313
closes #178

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.

CalMacCQ · 2024-09-17T14:30:32Z

pytket/extensions/qiskit/qiskit_convert.py

+    else:
+        base_tket_gate: OpType = _known_qiskit_gate[c_gate.base_gate.base_class]
+
+        base_op: Op = Op.create(base_tket_gate, params)  # type: ignore


Maybe I can get around this type: ignore. Mypy doesn't seem happy with the fact that base_op is defined on two branches.

The _get_unitary_box function has return type Unitary1qBox | Unitary2qBox | Unitary3qBox. Not sure if mypy is able to find out that these unitary boxes inherit from Op.

CalMacCQ · 2024-09-17T15:17:54Z

tests/qiskit_convert_test.py

@@ -865,6 +867,21 @@ def test_controlled_unitary_conversion() -> None:
    assert np.allclose(u_qc, u_tkc)


+def test_qcontrol_box_conversion_to_qiskit() -> None:


I think I will add tests for a wider range of controlled operations

Yes please.

Conversion doesn't work properly for various parameterized gates. Will update the handling and add tests.

I've fixed the conversion of parametrised gates in b42946e and added some controlled parametrised gates to the test in c1b315c

This reverts commit 16453b1.

tests/qiskit_convert_test.py

pytket/extensions/qiskit/qiskit_convert.py

cqc-alec · 2024-09-18T08:19:48Z

tests/qiskit_convert_test.py

@@ -865,6 +867,21 @@ def test_controlled_unitary_conversion() -> None:
    assert np.allclose(u_qc, u_tkc)


+def test_qcontrol_box_conversion_to_qiskit() -> None:


Yes please.

CalMacCQ · 2024-09-23T10:59:40Z

tests/qiskit_convert_test.py

+    DecomposeBoxes().apply(circ1)
+    DecomposeBoxes().apply(circ2)
+    assert circ1 == circ2
+    assert compare_unitaries(circ1.get_unitary(), circ2.get_unitary())


I think this check of unitary equivalence may be redundant given the previous line

Yes, can remove this. However, I think we should add a test that compares the unitary of the tket circuit with the unitary of the qiskit circuit (adjusted for endianness).

added in 05dd936

cqc-alec · 2024-09-24T07:32:59Z

tests/qiskit_convert_test.py

+    DecomposeBoxes().apply(circ1)
+    DecomposeBoxes().apply(circ2)
+    assert circ1 == circ2
+    assert compare_unitaries(circ1.get_unitary(), circ2.get_unitary())


Yes, can remove this. However, I think we should add a test that compares the unitary of the tket circuit with the unitary of the qiskit circuit (adjusted for endianness).

CalMacCQ added 3 commits September 12, 2024 10:53

simple converter function

c32e2ee

start adding control state handling

77c5d97

implement control_state handling in qiskit_to_tk

0eb0610

CalMacCQ requested a review from cqc-melf as a code owner September 12, 2024 15:35

CalMacCQ marked this pull request as draft September 12, 2024 15:35

CalMacCQ changed the title ~~feat: directly handle control state in conversion of controlled gates~~ feat: handle control state in conversion of controlled gates Sep 12, 2024

CalMacCQ added 19 commits September 12, 2024 16:43

remove unecessary if-else

903f2e9

use a list comprehension

fdc0c4e

annotate helper function call

9fe2128

refactor qcontrolbox building out of _add_qiskit_data

5dab332

update controlbox utility function

0feb9fd

fix ordering in Unitary boxes

6f24ed8

get rid of Union typing

5038ea5

use helper function to handle UnitaryGate instances

73c9ab8

fix handling for all UnitaryGate instances

c39b82b

remove original UnitaryGate helper function

7e8b770

helper function takes a unitary as an argument not a UnitaryGate

f3761d1

add assert and a comment

0aefdfc

Support QControlBox in tk_to_qiskit converter

b2b48b0

add test for QControlBox conversion

cd69dff

shorten line to make pylint happy

0ab2d48

check unitary equivalence in test

57d55fd

add a type ignore for mypy for now

a97d407

remove a type: ignore comment

cd9cec9

unitarybox helper takes num_qubits as an arg

62ac814

CalMacCQ commented Sep 17, 2024

View reviewed changes

CalMacCQ added 3 commits September 17, 2024 15:49

add another comment

8cc30ac

add changelog entry

700dd55

add another changelog entry

7e3dfe6

CalMacCQ changed the title ~~feat: handle control state in conversion of controlled gates~~ feat: Improved handling of controlled gates in converters Sep 17, 2024

CalMacCQ commented Sep 17, 2024

View reviewed changes

CalMacCQ marked this pull request as ready for review September 17, 2024 15:18

CalMacCQ requested a review from cqc-alec September 17, 2024 15:19

CalMacCQ added 3 commits September 17, 2024 16:25

remove duplicate assert

cb81a07

update docs build in build and test workflow

16453b1

Revert "update docs build in build and test workflow"

5b4aeec

This reverts commit 16453b1.

cqc-alec reviewed Sep 18, 2024

View reviewed changes

CalMacCQ removed the request for review from cqc-melf September 18, 2024 09:12

CalMacCQ added 4 commits September 18, 2024 12:47

import _get_pytket_ctrl_state following review

daa3090

fix handling of parameterised gate

b42946e

add a couple of tests for parameterised gates

c1b315c

fix import

4473235

CalMacCQ commented Sep 23, 2024

View reviewed changes

Merge branch 'main' into feat/improved_controlled_gates

9dee730

CalMacCQ requested a review from cqc-alec September 23, 2024 12:43

cqc-alec requested changes Sep 24, 2024

View reviewed changes

update unitary testing

05dd936

cqc-alec approved these changes Sep 24, 2024

View reviewed changes

CalMacCQ merged commit e148bd8 into main Sep 24, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Improved handling of controlled gates in converters #391

feat: Improved handling of controlled gates in converters #391

CalMacCQ commented Sep 12, 2024 •

edited

Loading

CalMacCQ Sep 17, 2024

CalMacCQ Sep 17, 2024

cqc-alec Sep 18, 2024

CalMacCQ Sep 18, 2024 •

edited

Loading

CalMacCQ Sep 23, 2024 •

edited

Loading

cqc-alec Sep 18, 2024

CalMacCQ Sep 23, 2024

cqc-alec Sep 24, 2024

CalMacCQ Sep 24, 2024

cqc-alec Sep 24, 2024

		@@ -865,6 +867,21 @@ def test_controlled_unitary_conversion() -> None:
		assert np.allclose(u_qc, u_tkc)


		def test_qcontrol_box_conversion_to_qiskit() -> None:

feat: Improved handling of controlled gates in converters #391

feat: Improved handling of controlled gates in converters #391

Conversation

CalMacCQ commented Sep 12, 2024 • edited Loading

Description

Related issues

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CalMacCQ Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

CalMacCQ Sep 23, 2024 • edited Loading

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

CalMacCQ commented Sep 12, 2024 •

edited

Loading

CalMacCQ Sep 18, 2024 •

edited

Loading

CalMacCQ Sep 23, 2024 •

edited

Loading