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

Qiskit Conversion ignores implict permutations #411

Closed
christian512 opened this issue Nov 4, 2024 · 7 comments · Fixed by #412 or #421
Closed

Qiskit Conversion ignores implict permutations #411

christian512 opened this issue Nov 4, 2024 · 7 comments · Fixed by #412 or #421
Assignees
Labels
bug Something isn't working circuit_conversion Issues and pull requests related to coverting qiskit circuits to pytket and vice versa

Comments

@christian512
Copy link

If a Circuit object contains implicit_permutations the conversion using tk_to_qiskit ignores these permutations leading to an inequivalent circuit (see example below).

I would suggest to (at least) raise a warning that the conversion function is called on a circuit containing implicit permutations.

from pytket import Circuit
from pytket.extensions.qiskit import qiskit_to_tk, tk_to_qiskit
from pytket.utils import compare_statevectors

circuit_dict = {'bits': [],
     'commands': [
         {'args': [['q', [0]]], 'op': {'type': 'X'}},
         {'args': [['q', [1]]], 'op': {'type': 'Y'}},
         {'args': [['q', [2]]], 'op': {'type': 'Z'}},],
         'implicit_permutation': [[['q', [0]], ['q', [0]]],
                                  [['q', [1]], ['q', [2]]],
                                  [['q', [2]], ['q', [1]]]],
         'phase': '0.0',
         'qubits': [['q', [0]], ['q', [1]], ['q', [2]]]
    }

circ = Circuit.from_dict(circuit_dict)

# Pass the circuit through qiskit conversion
qiskit_circ = tk_to_qiskit(circ)
print(qiskit_circ)
circ_after_conversion = qiskit_to_tk(qiskit_circ)

# This statevector comparison fails
assert compare_statevectors(circ_after_conversion.get_statevector(), circ.get_statevector())
@cqc-alec cqc-alec transferred this issue from CQCL/tket Nov 4, 2024
@CalMacCQ CalMacCQ added bug Something isn't working circuit_conversion Issues and pull requests related to coverting qiskit circuits to pytket and vice versa labels Nov 4, 2024
@CalMacCQ
Copy link
Contributor

CalMacCQ commented Nov 4, 2024

Thanks for raising this issue. This is definetly something we should address.

@CalMacCQ
Copy link
Contributor

CalMacCQ commented Nov 5, 2024

Maybe just emitting a warning saying that the circuit contains implcit permutations is the best way to tackle this. Any thoughts @cqc-alec ?

@CalMacCQ CalMacCQ self-assigned this Nov 6, 2024
@cqc-alec
Copy link
Collaborator

cqc-alec commented Nov 7, 2024

Note that there is also a replace_implicit_swaps argument to tk_to_qiskit that implements the implicit permutation using swaps. But yes, I think emitting a warning if this is omitted (or False) and the circuit has a non-trivial implicit permutation is a good idea.

@CalMacCQ
Copy link
Contributor

CalMacCQ commented Nov 20, 2024

Adding this warning actually had an unintended consequence that I should've anticipated. Namely that the different backends in pytket-qiskit use the tk_to_qiskit function on circuits that have implcit swaps introduced by passes in optimisation level=2.

The statevector and unitary backends handle these swaps in postprocessing so the results are still correct. This does however spam the user with unhelpful warning messgaes which they are not able to address directly.

We could make the backends handle this by adding swap gates but adding additonal gates would not be desirable for backends with quantum noise.

GIven the above, I'm thinking I'll revert #412 for now. Maybe we should just add a note in the docs for this.

@christian512
Copy link
Author

What about an option to deactivate the warnings?

The backends could use tk_to_qiskit(..., warn=False) where warn is an optional parameter that defaults to True.

@CalMacCQ
Copy link
Contributor

CalMacCQ commented Nov 21, 2024

What about an option to deactivate the warnings?

The backends could use tk_to_qiskit(..., warn=False) where warn is an optional parameter that defaults to True.

I did think this as an option too. Could be worth doing and I think it should work to give the warnings where we want them.

@CalMacCQ
Copy link
Contributor

I'll add the warning back in with the flag. Should be quick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working circuit_conversion Issues and pull requests related to coverting qiskit circuits to pytket and vice versa
Projects
None yet
3 participants