CQCL / pytket-qiskit

pytket-qiskit, extensions for pytket quantum SDK
Apache License 2.0
17 stars 13 forks source link

Qiskit Conversion ignores implict permutations #411

Closed christian512 closed 2 days ago

christian512 commented 2 weeks ago

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())
CalMacCQ commented 2 weeks ago

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

CalMacCQ commented 2 weeks ago

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

cqc-alec commented 2 weeks ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 2 days ago

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