Calibrations Definition

[eggerdj]

Summary

The CalibrationsDefinition class is intended to let users manage parameters and schedules.

Details and comments

At the heart of the calibrations definition is a dict of template schedules and a dict of parameters with values. The template schedules are fully parametric schedules (i.e. pulse durations, pulse parameters, channel indices, etc... are all parameters). The dict of parameters stores the values of the each parameter for different qubits and schedules as the parameter evolves through the calibration.

[wshanks]

This looks good. It is nice to break it out from #20. The biggest questions I have right now are:

1. Should we allow different schedules with the same name for different qubits? I made an inline comment about this question.
2. Will working with qiskit.circuit.Parameters like this work okay? It seems like it will be okay, but it also seems like there is some potential for user error because the links between parameters in different schedules is somewhat hidden (compared to the previous global parameter name method where a user would very explicitly use a amp_xp in both places) and I could see someone accidentally using a new amp variable in xm instead of reusing the one used in xp. I think that might just be something one gets used to though.
3. How does serializing and deserializing the calibrated parameters work? I think storing values indexed as (schedule name, parameter name, qubits): value and then using add_parameter_value to load them back should work. Partly I wonder about this because Calibrations stores the values internally keyed off of Parameter instances which don't live outside of one Python process, but add_parameter_value looks up the Parameter instances so it should be fine.

[wshanks]

Could we allow for different qubits to use different schedules for the same schedule name? It would be nice to retain the default behavior of not specifying qubits and having the schedule apply to all qubits that have not had specific schedules set for them, but sometimes you might want to vary the schedule for different qubits. This would require tweaking the way parameters are stored in the class right now.

[nkanazawa1989]

This is good point. It sounds like users need to manage schedule name though there is almost no flexibility for the schedule name, i.e I guess the name is used to map the gate to schedule, or basis_gate name. Perhaps you need another dict that maps (Gate, qubits) to schedule.

[wshanks]

I am wondering about the case where the user adds a schedule name that was already added previously. I suppose it is the user's responsibility to reuse the same Parameter as the previous schedule to avoid losing the history of values stored under the old Parameter that now won't be pointed to by (schedule.name, param.name)?

[eggerdj]

Perhaps we can make this easier by adding a flag which makes the decision a conscious one? E.g. change the signature of the method to:

def add_schedules(self, schedules: Union[Schedule, List[Schedule]], override_existing: bool = False):

Then if the schedule name is already present and override_existing is False we raise an error. We could also check the hash of the parameters for existing schedule to avoid history loss. How does that sound?

[wshanks]

Depending on the work flow, it might be sufficient to forbid a schedule to be added twice. If it is easy to save to and reload from the database, the answer might be that one should create a new CalibrationsDefinition instance and reload if one wants to redefine schedules. On the other hand, maybe it is something that a user would expect to be able to do, so it should be allowed.

I don't think it needs by default to be an error for a user to redefine a schedule. It is when a user replaces a Parameter that I was concerned about. The storage of parameters is (schedule.name, parameter.name): Parameter in _parameter_map and Parameter: {qubits: [ParameterValue, ...]} in _params. So if (schedule.name, parameter.name) is updated to point to a new Parameter in _parameter_map, that (schedule.name, parameter.name) loses the history stored in the old Parameter entry in _params. I think this is how it worked when I made the comment above. Now with the way you wrote register_parameter, a CalibrationError will be raised for the second schedule if it reuses any of the parameter names of the first. register_parameter(p, s) probably should not raise an exception if p == _parameter_map[(s.name, p.name)] (re-adding a schedule with the same parameter instances).

What did you mean by "check the hash?" Do you mean that if (schedule.name, parameter.name) is already in _parameter_map that _params[_parameter_map[(schedule.name, parameter.name)]] would be copied to _params[parameter] if parameter != _parameter_map[(schedule.name, parameter.name)]? That seems like a natural thing to do. It allows for messy situations though where (xp, amp) and (yp, amp) point to the same Parameter in _params but then calling add_schedules(xp) a second time with a new amp Parameter results in xp and yp pointing to separate entries in _params.

I think this is somewhat of an edge case concern, but it is a question of how much flexibility to give the user vs keeping the implementation simple and giving the user less chance to get confused.

[eggerdj]

Part of this is addressed in 72ce1c0. We still have the issue of tracking the history of the links between parameters and schedules when schedule are re-added.

[nkanazawa1989]

New implementation looks nice. Perhaps we need to reconsider what defines the scope of parameter.

[nkanazawa1989]

This is good point. It sounds like users need to manage schedule name though there is almost no flexibility for the schedule name, i.e I guess the name is used to map the gate to schedule, or basis_gate name. Perhaps you need another dict that maps (Gate, qubits) to schedule.

[CLAassistant]

All committers have signed the CLA.