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 delay gates do not handle unit='s' #149

Open
eendebakpt opened this issue Oct 18, 2022 · 4 comments
Open

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

eendebakpt opened this issue Oct 18, 2022 · 4 comments

Comments

@eendebakpt
Copy link
Contributor

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
Copy link

jwoehr commented Oct 18, 2022

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
Copy link
Contributor Author

The assemble is called here:

qobj = assemble(run_input, self, **run_config_dict)

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
Copy link

jwoehr commented Oct 18, 2022

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

@QFer
Copy link
Contributor

QFer commented Feb 28, 2023

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

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

No branches or pull requests

3 participants