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

Conversation

CalMacCQ
Copy link
Contributor

@CalMacCQ CalMacCQ commented Nov 7, 2024

Description

Minor change to tk_to_qiskit. The user now gets a warning if the input pytket Circuit contains implicit qubit permutations.

closes #411

Related issues

#411

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 CalMacCQ requested a review from cqc-melf as a code owner November 7, 2024 13:58
@CalMacCQ CalMacCQ requested review from cqc-alec and removed request for cqc-melf November 7, 2024 13:59
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.

@@ -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.

pytket/extensions/qiskit/qiskit_convert.py Outdated Show resolved Hide resolved
@CalMacCQ CalMacCQ requested review from cqc-alec and removed request for cqc-alec November 8, 2024 10:31
@CalMacCQ CalMacCQ merged commit ffbf31a into main Nov 8, 2024
6 checks passed
@CalMacCQ CalMacCQ deleted the warn/impl_q_perm branch November 26, 2024 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Qiskit Conversion ignores implict permutations
2 participants