Calibration module

[eggerdj]

Summary

This is the first PR for the calibration module.

Details and comments

The PR introduces one calibration experiment RoughAmplitude, the classes needed to store calibration schedules and parameters, the base class for calibration analysis (derived from BaseAnalysis), and other classes needed by the calibration module.

TODOs

• Improve tests.
• Align with Curve fitting analysis helper functions #19

[yaelbh]

If the current code is good then it's better to merge it, and adjust it to #19 when #19 is ready, rather than wait for #19.

[eggerdj]

I'm fine with merging without adjusting to #19. Furthermore, #19 may benefit from the DataProcessor in this PR. One round of feedback on the high-level implementation is however still needed here.

[nkanazawa1989]

Thanks Daniel for excellent work. I think this is really good start of discussion. I also think data processing chain should be exported as a standard data acquisition logic of the qiskit experiment. Perhaps it's better to separate it from this PR.

[nkanazawa1989]

We should define a class variable

__calibration_objective__ = {
    'gates': ['x90p', 'xp'],
    'parameter_name: 'amp'
}

This template should be defined in the superclass. Then gate_name and parameter_name will be optional. This is closer to the user experience of backend codebase. Also note that the rough amplitude can update multiple gates, so signature should be list of str.

[eggerdj]

This is a good idea but needs to be extended. In your example we also need to specify the angle for the rabi calibration. I suggest adding an extra field called options so that your example above becomes:

__calibration_objective__ = {
    'gates': ['x90p', 'xp'],
    'options': [np.pi/2, np.pi],
    'parameter_name: 'amp'
}

[eggerdj]

See c67acbd

[eggerdj]

See e.g. fa3f3eb for a follow up.

[eggerdj]

Let's go for an instance variable.

[nkanazawa1989]

Users need to manage parameter names for every schedule so that not overlapping with existing name, when constructing schedule? I think this is very annoying.

[eggerdj]

I think this is acceptable as in most cases we only need a very limited number of schedules. If you have a better alternative to suggest please do so.

[nkanazawa1989]

In our conventional database this has been organized as somewhat like Dict[qubit(s)][pulse name][param name]. Do you think this new data structure, i.e. Dict[param name][channel] is reasonable? @wshanks

[wshanks]

It does feel like some structure is lost going with just (channel, name) rather than (channel, schedule/gate name, parameter name). The result of this is that the parameter names end up getting both the gate and the name stuck together (like amp_xp).

I thought the main reason that globally unique parameter names were required was that Schedule.assign_parameters replaces parameters in a schedule with their values, so the schedule must be deep copied before calling assign_parameters and that would create new Parameter instances with new UUID's. However, looking through the qiskit-terra code, I see that Parameter bypasses __copy__ and __deepcopy__, so the UUID's are actually preserved: https://github.com/Qiskit/qiskit-terra/blob/bdc73936d626d14057e0c95e7daaa25b7490020f/qiskit/circuit/parameter.py#L69-L73 So maybe parameter UUID's could be used?

Also, I see that Schedule.assign_parameters actually creates a new pulse with substituted parameters replaced with their values: https://github.com/Qiskit/qiskit-terra/blob/bdc73936d626d14057e0c95e7daaa25b7490020f/qiskit/pulse/parameter_manager.py#L212-L217 I note this because I had thought it might just add a value into the Parameter object that would affect other schedules using that Parameter but it does not.

One benefit of using parameter UUID's is that parameters could be linked across schedules if a table of UUID: (channel, name) or UUID: (channel, schedule name, parameter name) were kept. In the current implementation, parameter substitution is based on channel plus name, so you can't use the same parameter in schedules on two different channels. If the UUID's could be used, CalibrationsDefinition could add to a table of UUID: [(channel, schedule, parameter), ...] each time a schedule is added and then when generating circuits it could find which (channel, schedule, parameter) tuple has a calibrated value in CalibrationsDefinition's table of calibrated values and assign that value to uses of the Parameter with that UUID. One potential problem: how to handle the ambiguous case where there are calibrated values in CalibrationsDefinition for two different (channel, schedule name, parameter name) tuples that have the same Parameter instance?

I have been trying to think of what the trade offs are with using globally unique names per channel versus scoping the names within a schedule name. Here is what I thought of:

• Since parameters will mostly be specific to a schedule, scoping names to a schedule enforces more consistent naming: (channel=0, schedule=xp, parameter=amp) vs. possible confusion of amp_xp vs. xp_amp.
• Scoping to a schedule allows one to query the parameter database for all parameters associated with a specific schedule.
• I thought there might be some benefit to using generic parameter names (like amp and angle) to let the user define generic schedules to be reused for different gates but in practice schedules get built by code and that code could be put in a loop or a function to generate copies of the schedule with different parameters, so the parameter names don't matter.
• What about parameters that are not associated with a specific schedule like qubit_freq? Perhaps there could be some default all label for parameters not associated with a specific schedule. Otherwise that case is more straightforward for the global name case.

[nkanazawa1989]

Why this is needed? Seems like this is due to parameter name is not enough unique, i.e. add_parameter_value requires channel index. Dictionary key can only uniquely identify parameter, and name attached to parameter is defined in global scope.

We can internally rename attached parameter when schedule is added, or internally define mapping of unique parameter name to Parameter object uuid.

[eggerdj]

We cannot rely on Parameter UUID since the template schedules and their parameters are copied when generating new schedules with parameter values assigned. I would have much preferred to use UUIDs to make the parameters unique but this is not possible with the way parameters function in Qiskit.

Independently of UUIDs, get_channel_index is needed to accommodate control channels. Consider the CR gate schedule and write it out using schedules where all channel indices are parameters.

[nkanazawa1989]

Instead of defining free_params (i.e. experimental class needs to search for parameter name, since naming is up to users), we can generate value dict and use new pulse.call function behavior in the builder to bind parameters on the fly from the parameter database.

[eggerdj]

Can you elaborate on how this would work?

[wshanks]

This is pretty similar to the existing code stack. One thing it does that is not done here is interpolate the data (here it is assumed that the xvals are equally spaced). Also, it takes a few points around the peak in the fft and fits a lorentzian to that to fine tune the frequency. I don't know how important that is.

[wshanks]

In the existing code stack, initial guess and fit function are grouped and I haven't noticed anyone needing to deviate from that. A generic function could be factored out of the fit_function method and reused by multiple classes if it is found to be useful to share it.

The examples here could all be static methods, but I suppose in general an analysis class could have options (like a polynomial analysis class that takes a degree as an argument).

[wshanks]

Why not make get_period_fraction and get_min_location be methods of the Analysis class since they are implementing behavior on a per-type basis already?

Something like @nkanazawa1989's suggestion would also work. I don't know if you want to specify all of a fit's parameters, but you at least want to specify "this analysis returns a frequency value."

[wshanks]

It does feel like some structure is lost going with just (channel, name) rather than (channel, schedule/gate name, parameter name). The result of this is that the parameter names end up getting both the gate and the name stuck together (like amp_xp).

I thought the main reason that globally unique parameter names were required was that Schedule.assign_parameters replaces parameters in a schedule with their values, so the schedule must be deep copied before calling assign_parameters and that would create new Parameter instances with new UUID's. However, looking through the qiskit-terra code, I see that Parameter bypasses __copy__ and __deepcopy__, so the UUID's are actually preserved: https://github.com/Qiskit/qiskit-terra/blob/bdc73936d626d14057e0c95e7daaa25b7490020f/qiskit/circuit/parameter.py#L69-L73 So maybe parameter UUID's could be used?

Also, I see that Schedule.assign_parameters actually creates a new pulse with substituted parameters replaced with their values: https://github.com/Qiskit/qiskit-terra/blob/bdc73936d626d14057e0c95e7daaa25b7490020f/qiskit/pulse/parameter_manager.py#L212-L217 I note this because I had thought it might just add a value into the Parameter object that would affect other schedules using that Parameter but it does not.

One benefit of using parameter UUID's is that parameters could be linked across schedules if a table of UUID: (channel, name) or UUID: (channel, schedule name, parameter name) were kept. In the current implementation, parameter substitution is based on channel plus name, so you can't use the same parameter in schedules on two different channels. If the UUID's could be used, CalibrationsDefinition could add to a table of UUID: [(channel, schedule, parameter), ...] each time a schedule is added and then when generating circuits it could find which (channel, schedule, parameter) tuple has a calibrated value in CalibrationsDefinition's table of calibrated values and assign that value to uses of the Parameter with that UUID. One potential problem: how to handle the ambiguous case where there are calibrated values in CalibrationsDefinition for two different (channel, schedule name, parameter name) tuples that have the same Parameter instance?

I have been trying to think of what the trade offs are with using globally unique names per channel versus scoping the names within a schedule name. Here is what I thought of:

• Since parameters will mostly be specific to a schedule, scoping names to a schedule enforces more consistent naming: (channel=0, schedule=xp, parameter=amp) vs. possible confusion of amp_xp vs. xp_amp.
• Scoping to a schedule allows one to query the parameter database for all parameters associated with a specific schedule.
• I thought there might be some benefit to using generic parameter names (like amp and angle) to let the user define generic schedules to be reused for different gates but in practice schedules get built by code and that code could be put in a loop or a function to generate copies of the schedule with different parameters, so the parameter names don't matter.
• What about parameters that are not associated with a specific schedule like qubit_freq? Perhaps there could be some default all label for parameters not associated with a specific schedule. Otherwise that case is more straightforward for the global name case.

[wshanks]

Looking at this makes me wonder if add_parameter_value() could also accept numeric values and cast them ParameterValue().

Also, perhaps there could be a function that takes a dict like {'drive': {0: {name: val, ...}, 1: {...}}, 'measure': {0: ...}, 'control': {'0.1': {name: val, ...}}} for quickly populating initial parameters. Though maybe to_db/from_db for local files will be better for that.

[wshanks]

Should the qubit_freq parameter get used in some way within this PR? When in the code is it used to set the frequency?

[wshanks]

So does the user have to call this separately after calling run()? Partly I am wondering how BaseCalibrationExperiment combines with ParallelExperiment to allow running a calibration on multiple qubits at the same time.

[eggerdj]

Replaced by PR #26