Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Warn user about implicit qubit permutations in tk_to_qiskit #412

Merged
merged 8 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
## Unreleased

- Fix handling of non-default registers when selecting bits in results.
- The {py:func}`tk_to_qiskit` converter gives a warning if the input {py:class}`~pytket.circuit.Circuit` contains [implicit qubit permutations](https://docs.quantinuum.com/tket/user-guide/manual/manual_circuit.html#implicit-qubit-permutations).

## 0.58.0 (October 2024)

Expand Down
15 changes: 15 additions & 0 deletions pytket/extensions/qiskit/qiskit_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

"""Methods to allow conversion between Qiskit and pytket circuit classes
"""
import warnings
from collections import defaultdict
from collections.abc import Iterable
from inspect import signature
Expand Down Expand Up @@ -855,6 +856,11 @@ def append_tk_command_to_qiskit(
supported_gate_rebase = AutoRebase(_protected_tket_gates)


def _has_implicit_permutation(circ: Circuit) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be better defined in pytket rather than this extension: there's nothing specific to qiskit about it. But for now there is no need to factor it out as a method, since it's only used in one place.

Copy link
Contributor Author

@CalMacCQ CalMacCQ Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Maybe it should just be a property/method of Circuit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But for the purpose of this PR the check can just be inlined.

impl_dict: dict[Qubit, Qubit] = circ.implicit_qubit_permutation()
return tuple(impl_dict.keys()) != tuple(impl_dict.values())
CalMacCQ marked this conversation as resolved.
Show resolved Hide resolved


def tk_to_qiskit(
tkcirc: Circuit, replace_implicit_swaps: bool = False
) -> QuantumCircuit:
Expand All @@ -873,6 +879,15 @@ def tk_to_qiskit(
tkc = tkcirc.copy() # Make a local copy of tkcirc
if replace_implicit_swaps:
tkc.replace_implicit_wire_swaps()

if _has_implicit_permutation(tkcirc) and not replace_implicit_swaps:
warnings.warn(
"The pytket Circuit contains implicit qubit permutations"
+ " which aren't handled by default."
+ " Consider using the replace_implicit_swaps flag in tk_to_qiskit or"
+ " replacing them using Circuit.replace_implicit_swaps()."
)

qcirc = QuantumCircuit(name=tkc.name)
qreg_sizes: dict[str, int] = {}
for qb in tkc.qubits:
Expand Down
7 changes: 7 additions & 0 deletions tests/qiskit_convert_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1172,3 +1172,10 @@ def test_nonregister_bits() -> None:
c.rename_units({Bit(0): Bit(1)})
with pytest.raises(NotImplementedError):
tk_to_qiskit(c)


def test_implicit_swap_warning() -> None:
c = Circuit(2).H(0).SWAP(0, 1)
c.replace_SWAPs()
with pytest.warns(UserWarning, match="The pytket Circuit contains implicit qubit"):
Copy link
Contributor Author

@CalMacCQ CalMacCQ Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindof suprised that python can use this UserWarning class without it being imported.

I guess errors like NotImplementedError don't need to be imported either so its not so surprising.

tk_to_qiskit(c)