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

Add pulse gate pass #6759

Merged
merged 30 commits into from
Aug 31, 2021
Merged

Conversation

nkanazawa1989
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 commented Jul 16, 2021

Summary

This PR improves handling of pulse gates.

Current work flow

# write custom gate
with pulse.build(name="my_gate") as x_gate:
    ...

circ1 = QuantmCircuit(1)
circ1.x(0)
circ1.delay(10, 0)
circ1.measure_all()

circ2 = QuantmCircuit(1)
circ2.x(0)
circ2.delay(15, 0)
circ2.measure_all()

# we need to repeat this line for all circs
circ1.add_calibration(gate="x", qubits=(0,), schedule=x_gate)
circ2.add_calibration(gate="x", qubits=(0,), schedule=x_gate)

circs = transpile([circ1 + circ2], backend)
backend.run(circs)

With new pass

with pulse.build(name="my_gate") as x_gate:
    ...

backend.defaults().instruction_schedule_map.add(instruction="x", qubits=(0,), schedule=x_gate)

circ1 = QuantmCircuit(1)
circ1.x(0)
circ1.delay(10, 0)
circ1.measure_all()

circ2 = QuantmCircuit(1)
circ2.x(0)
circ2.delay(15, 0)
circ2.measure_all()

# instmap is automatically extracted from backend and x gate schedule is overridden
circs = transpile([circ1 + circ2], backend)
backend.run(circs)

This may benefit the calibration framework in Qiskit Experiment.

Details and comments

The simple trick enabling this workflow is the schedule metadata "publisher" (name TBD, welcome suggestions). Note that schedules in the instmap that are provided by the backend are always registered by PulseDefaults constructor. This PR adds the publisher metadata of backend to the schedule registered by this.

https://github.com/Qiskit/qiskit-terra/blob/9b48476cbaf5c4742f67e1ff6ce2546ba5fad01f/qiskit/providers/models/pulsedefaults.py#L204-L208

If InstructionScheduleMap.add is called by users, this will implicitly adds metadata of client (since we assume such schedule doesn't have metadata). This enables the pass to distinguish user-defined calibration from the backend provided calibrations. Thanks to this logic, we can selectively add user defined calibrations to the circuit.

TODO

  • Finalize the mechanism to distinguish backend/user-defined cals
  • Write unittest
  • Write reno

@nkanazawa1989
Copy link
Contributor Author

Thanks @eggerdj for the reviewing. I think this pass is necessary for all optimization levels, because calibration update and circuit optimization are separate context. For example, user should be able to create custom x gate, and use that gate with every optimization levels. Basically this is the same as writing :

non_opt_circ = QuantumCircuit(nq)
...
non_opt_circ.add_calibration("x", (0,), schedule=x_gate)

opt0 = transpile(non_opt_circ, backend, optimization_level=0)
opt1 = transpile(non_opt_circ, backend, optimization_level=1)
opt2 = transpile(non_opt_circ, backend, optimization_level=2)
opt3 = transpile(non_opt_circ, backend, optimization_level=3)

however here we want to avoid explicitly writing add_calibration for each circuit. If we calibrate all basis gates, it would be very annoying to manually overriding all of quantum gates in the circuit.

@nkanazawa1989 nkanazawa1989 marked this pull request as ready for review July 20, 2021 02:41
@ajavadia
Copy link
Member

There are two things that make me feel a bit uneasy about this approach:

1- The idea of changing backend.defaults(), only to be able to leverage transpile(circ, backend) later, is inconsistent with the rest Qiskit (I'm not aware of backend being changed elsewhere).
In transpile(), the idea is that all info will be pulled from backend, but they can be over-ridden by other arguments (e.g. basis_gates, coupling_map, instruction_durations). So if I want to change the durations I don't change those of the backend, I supply my own instruction_durations like this:

from qiskit.transpiler import InstructionDurations
durations = InstructionDurations.from_backend(backend)
durations.update('cx', (0, 1), 200, 'ns')
transpile(circ, backend, durations)  # this will over-ride the backend's durations

2- Is it really a big deal to write add_calibration for the circuits? If there are multiple calibrations you can put the in a add_calibrations() function and just write one line per circuit. This way at least it's explicit what's happening.

@nkanazawa1989
Copy link
Contributor Author

nkanazawa1989 commented Jul 20, 2021

Thanks Ali for the comments :)

1- The idea of changing backend.defaults(), only to be able to leverage transpile(circ, backend) later, is inconsistent with the rest Qiskit (I'm not aware of backend being changed elsewhere).

I don't remember where I saw this, but if I recall correctly

  • backend.configurations: immutable property.
  • backend.properties: property that can be updated with calibration.
  • backend.defaults: ptoperty that can be updated with calibration, and user can override.

According to this API docs, defaults is

Description of default settings for Pulse systems. These are instructions or settings that may be good starting points for the Pulse user. The user may modify these defaults for custom scheduling.

so overriding backend defaults is not quite unexpected usage. Actually, almost all members of PulseDefaults are mutable, so they are implicitly updated unless we deepcopy them when accessing.
Note that I added inst_map argument to transpiler, so you can still override this with user provided argument.

2- Is it really a big deal to write add_calibration for the circuits?

Yes. With above code example it's not quite easy to understand the benefit of this implementation (you can still write add_calibration manually). However, this change will drastically improves user experience in qiskit-experiment and other qiskit modules that may use custom gate.

Let's assume following workflow. Here we want to calibrate SX gate and evaluate this with randomized benchmarking. Please compare the pseudo codes below.

in current workflow

define calibration

# run set of calibrations to define basis_gates
for cal_exp in calibration_experiments:
   result = cal_exp.run(backend)
   calibrations.update(result)

import rb
circuits = transpile(rb.circuits(backend), backend)  # we need to insert hook to add cals
for circ in circuits: 
   for q in circ.qubits:
     circ.add_calibration("sx", (q,), calibrations.get_schedule("sx", (q, )))

backend.run(circs)

what this PR proposes

define calibration

# run set of calibrations to define basis_gates
for cal_exp in calibration_experiments:
   result = cal_exp.run(backend)
   calibrations.update(result)

backend = calibrations.export()  # instmap is overridden with schedule with metadata

import rb
rb.run(backend)

The RB is a part of experiment so we can implement some calibration attachment logic in experiment. However, once we try to use calibrated gate for other expeirment, i.e. we may want to calibrate custom basis gates, such as CCR gate that implements iSWAP, and we may want use this for VQE. However, in this case, we need to write the hook shown in above example. This is really hurts user experience.

(edited)
The calibrations may update some other parameters such as qubit and resonator frequency. Of course we can avoid updating backend itself and export only instmap from the calibrations. However, one always need to manage multiple objects, i.e. instmap, qubit freqs, meas freqs etc... If some argument is missing in transpiler argument, one cannot reproduce calibrated gate with other experiment. So I prefer passing updated backend object.

@ajavadia
Copy link
Member

ajavadia commented Aug 3, 2021

As commented in #6825 I think this approach is fine.

This is a bit of me thinking out loud: I don't quite like this process of discovering which cals are supplied by the backend and which cals are supplied by the user seems a bit hacky (relying on 'metadata').

If I understand correctly you are doing this to attach user cals to the payload (circ.add_calibration). What if we change the payload model a bit and make all cals attached to the circuit? i.e. the payload contains instructions and all cals for those instructions. This makes the payload a bit larger, but I don't think by much if we're using parametric pulses. The advantage is that the payload is self-contained. Right now we have a frontend compiler and a backend compiler which means we can send the following 3 types of circuits:

  • circuit with physical gates/qubits
  • circuit with timing
  • circuit with pulses

This is a bit arbitrary IMO. We could even go one level higher and ask backend compiler do more compilation --- so we can even send a circuit with toffolis as payload (e.g. ionq does this). I think a better philosophy is to make the frontend compiler create the actual physical circuit, which inevitably means resolving timing and also attaching pulse definitions for all the gates. This type of payload is then consistent with qasm3 as well.

So even though the backend advertises its gate cals, we still include the subset of cals relevant to our circuit in the payload. It is then very explicit what the payload is doing (even if someone inspects it outside of the context of the backend).

@nkanazawa1989
Copy link
Contributor Author

nkanazawa1989 commented Aug 3, 2021

The advantage is that the payload is self-contained

I like the idea. Since usually backends don't define many kind of basis gates, the overhead of attaching all gate defs is not quite large. However this sounds like we need matrix-like configuration rather than current optimization_level, namely

| opt level   | 0 | 1 | 2 |
| target code |   |   |   |
|-------------+---+---+---| 
| qasm sim    | A | B | C |   circuit with physical gates/qubits (1)
| backend     | D | E | F |   circuit with timing + circuit with pulses (2)

because generation of lower layer target code (timing/gate-attached) may hurt simulator performance. This is rough idea, but we may still need to separate circuit with timing and circuit with pulses because some circuit simulation with special noise models (e.g. T1/T2/static-ZZ simulation) require scheduled circuit. I'm not sure how such simulations are sensitive to performance issue, but I know (1) is very sensitive to this problem since there are many competitors (actually going (1) -> (2) is computationally expensive since circuit scheduling regenerates DAG circuit, but attaching gate def is not quite expensive).

Anyhow unifying real backend execution pass to (2) seems to make the chain cleaner.

but I don't think by much if we're using parametric pulses

This is not true. In the instruction schedule map, pulse instructions are stored with assigned parameter because InstructionScheduleMap doesn't have a separate parameter table. Maybe this is worth considering since current implementation is designed before we have parametric pulses.

Another thing I don't quite understand is the behavior of backend compiler when calibration is attached to the circuit. If the compiler always recognizes the gate def entry as a custom pulse, it may duplicate the waveform data in the waveform memory, and eventually we could increase the risk of getting waveform memory overflow error.

@ajavadia
Copy link
Member

ajavadia commented Aug 3, 2021

generation of lower layer target code (timing/gate-attached) may hurt simulator performance

Simulator backends shouldn't have cals in them, so I don't think any cals get attached anyway.
They also don't report timing for gates, so again scheduling wouldn't be done.
But if it's a simulator that mimicks a device, then I think it's important to include e.g. delays in scheduling as that affects the noise model.

pulse instructions are stored with assigned parameter

Yeah definitely we should make those cals leverage parameterized pulses

@taalexander
Copy link
Contributor

taalexander commented Aug 5, 2021

What if we change the payload model a bit and make all cals attached to the circuit

I would be ok with this for a "pulse-level" payload.

We should not do this for all jobs (not sure if was being suggested) as:

  • This has a lot of duplication and will increase the size of the payload (most calibrations are shared across a job)
  • The pulse gate path is currently slower than the circuit only path and we will hurt performance on the backend for most users. While this will eventually be resolved, it will not be until the full release of QASM3 support.
  • Not all users/backends support pulse gates and so this information is redundant/not available.

I am not opposed to using the metadata for this purpose as you've done.
Another option would be to supply the override calibrations in a separate location and pull the override calibrations from that location. This would enable us to treat everything in that location as something that should be attached to the payload.

Personally, I have no problem with Backends being treated as mutable compilation targets. In the long run, I think they should even store/configure the passes that they will apply during thranspilation.

How do you determine whether to insert calibrations or not in transpile? Is there some option?

@nkanazawa1989
Copy link
Contributor Author

nkanazawa1989 commented Aug 5, 2021

Thanks Ali, Thomas,

Sounds like these comments are somewhat conflicting:

What if we change the payload model a bit and make all cals attached to the circuit

They also don't report timing for gates, so again scheduling wouldn't be done.

Perhaps we need tell the transpiler the level of compilation? i.e. simulation / hardware. this makes me think we need different quantum circuit classes depending on the stage of compilation.

supply the override calibrations in a separate location

If QASM3 code is generated per job we could serialize the instruction schedule map itself without attaching instructions to each circuit. In this case, how we can distinguish global defcal from one local to a circuit?

How do you determine whether to insert calibrations or not in transpile? Is there some option?

This is implicitly done by leveraging the metadata in current design. Because the inst map instance (attached to backend object) is populated with backend-calibrated schedules, it doesn't contain any user provided gate unless we explicitly instmap.add schedule. So the pulse gate pass checks the metadata of all gate schedules appearing in the circuit. By default, there is no user-provided schedule in the map and no calibration is attached. This could be a compute-heavy pass and maybe we should be able to enable/disable. On the other hand, this mean we always need to manage that flag and pass it down to the compilation chain -- this is actually I wanted to avoid.

I think a backend object is self-contaiend and can override basis gate only by itself.

@taalexander
Copy link
Contributor

Perhaps we need tell the transpiler the level of compilation? i.e. simulation / hardware. this makes me think we need different quantum circuit classes depending on the stage of compilation.

If QASM3 code is generated per job we could serialize the instruction schedule map itself without attaching instructions to each circuit. In this case, how we can distinguish global defcal from one local to a circuit?

This would require the ability to call defcals explicitly in the qasm3 and provide multiple calibration definitions for the same gate(which is planned). See openqasm/openqasm#244

@taalexander
Copy link
Contributor

I am ok with the metadata approach. Regarding multiple circuit classes, I would rather prefer to unify the circuit/pulse within one object structure (IR). We could have different canonical phases/forms within this IR structure to denote the format at the current point in compilation (eg., mapped, scheduled, calibrated, etc.).

…librations_pass

# Conflicts:
#	qiskit/compiler/transpiler.py
#	qiskit/transpiler/passes/scheduling/calibration_creators.py
…librations_pass

# Conflicts:
#	test/python/pulse/test_instruction_schedule_map.py
qiskit/compiler/transpiler.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/calibration/creators.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/calibration/creators.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/calibration/creators.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/calibration/creators.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/calibration/creators.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/calibration/creators.py Outdated Show resolved Hide resolved
qiskit/transpiler/preset_passmanagers/level0.py Outdated Show resolved Hide resolved
@mergify mergify bot merged commit 40deaf1 into Qiskit:main Aug 31, 2021
@nkanazawa1989 nkanazawa1989 mentioned this pull request Oct 13, 2021
8 tasks
@kdk kdk added this to the 0.19 milestone Nov 15, 2021
@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Dec 6, 2021
@1ucian0 1ucian0 mentioned this pull request May 13, 2022
@nkanazawa1989 nkanazawa1989 deleted the feature/calibrations_pass branch November 25, 2022 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants