QuTech-Delft / quantuminspire

Quantum Inspire SDK
Apache License 2.0
65 stars 27 forks source link

Qiskit delay gates do not handle unit='s' #149

Open eendebakpt opened 2 years ago

eendebakpt commented 2 years ago

When submitting qiskit circuits the unit attribute of the delay is ignored. E.g.

c=QuantumCircuit()
c.x(0)
c.delay(10, unit='dt')
c.delay(0.001, unit='s') # does not result in a 1 ms delay, but is rounded to 0

When a delay gate with unit 's' is used, it should be converted by quantuminspire to the appropriate equivalent in dt units. (e.g. for starmon-5 20e-9 seconds/cycle).

Using the dt unit when defining the delay is not always possible. For example when using circuits generated by third party code or when delays are connected to other parameters in the circuit (e.g. a phase).

@QFer

jwoehr commented 2 years ago

Altenrate delay units other than hardware cycles explicitly are not handled. From src/quantuminspire/qiskit/circuit_parser.py

def _delay(stream: StringIO, instruction: QasmQobjInstruction) -> None:
        """ Translates the delay element for a qubit. In cQASM wait parameter is int and the unit is hardware cycles.
        Only the Qiskit default unit "dt" will work correctly with cQASM, i.e. integer time unit depending on the
        target backend.

        In qiskit/circuit/delay.py multiple units are defined for delay instruction.
        In qiskit/circuit/instruction.py assemble() method the unit of the delay instruction is not passed. Only the
        parameter (which is the value of the delay instruction) is taken.
        Here we cannot convert delays originally in another unit than dt to dt, which is the unit for wait in cQASM.

        :param stream: The string-io stream to where the resulting cQASM is written.
        :param instruction: The Qiskit instruction to translate to cQASM.

        """
        wait_period = int(instruction.params[0])
        stream.write('wait q[{0}], {1}\n'.format(*instruction.qubits, wait_period))
eendebakpt commented 2 years ago

The assemble is called here: https://github.com/QuTech-Delft/quantuminspire/blob/9e401a96f9fec10821d36fb325c988f4909ed5bf/src/quantuminspire/qiskit/backend_qx.py#L176 Before that, quantuminspire could update the delay instructions to the appropriate units.

At the very least there should be a warning when non-integer values for the delay (and unit dt) are used.

Perhaps one could also look at the other qiskit backends how the delays are handled.

jwoehr commented 2 years ago

In any case, the Qiskit compatibility of QuantumInspire probably needs a good deal of work as things are changing rapidly in Qiskit.

QFer commented 1 year ago

@eendebakpt I will create 2 issues:

  1. At the very least there should be a warning when non-integer values for the delay (and unit dt) are used.
  2. quantuminspire should update the delay instructions to the appropriate units. The first issue is solvable. The second issue is only solvable when the backends propagate timing information (e.g., duration of a hardware cycle), which isn't currently the case and cannot be fixed in the quantuminspire SDK.