Calibrations fixes #740
Calibrations fixes #740
Conversation
- check consistency between coupling map and control channel map - made date times as time zone - added qubits arg to update inst map - fixed issue with qubits in update_inst_map
wshanks
left a comment
wshanks
left a comment
There was a problem hiding this comment.
I marked a few things to think about. Mainly it is the consistency test that we should think about before merging this because it points to a larger bug.
|
|
||
| self.assertEqual(self.cals.get_schedule("tcp", (3, 2)), expected) | ||
|
|
||
| def test_inst_map_stays_consistent(self): |
There was a problem hiding this comment.
I think it is a bug that this test is passing currently. The only reason it is passing is that the sigma parameters between the cr and xp schedules are tied, which is not realistic. If you uncouple the sigma parameters or switch to updating amp instead of sigma, the test fails.
When a parameter in a schedule nested inside a call instruction of an outer schedule is updated, the outer schedule also needs to be updated in the instruction schedule map. Should we address that case in this PR as well?
It also feels a bit weird that updating a parameter for a one qubit schedule also updates the parameter for two qubits. I thought we had scoped parameters to a number of qubits but perhaps not.
There was a problem hiding this comment.
Thanks for catching this. We should fix this in this PR. Here is how the test should look like: eace06e
There was a problem hiding this comment.
Ok this was non-trivial to fix. See here: f3b76b9
The complications were:
- figuring out which schedules call a schedule that we want to update and then add the caller to the update list.
- figuring out the qubits of the caller schedules. This happens in e.g. with an ECR gate that calls an X gate. Suppose we want to update
xpon qubit(3, ). Now suppose thatcron(3, 2)callsxpthen under step 1. we find that we need to also updatecrbut we only know that we have qubits(3, )we therefore need to figure out thatcrapplies, e.g., on(2, 3)and(3, 2). Out of safety both are updated here.
* Added extra test. * Added methodology to deal with parameter updating in Calls.
wshanks
left a comment
wshanks
left a comment
There was a problem hiding this comment.
Looks good! I had some minor comments.
Co-authored-by: Will Shanks <wshaos@posteo.net>
Co-authored-by: Will Shanks <wshaos@posteo.net>
Co-authored-by: Will Shanks <wshaos@posteo.net>
Co-authored-by: Will Shanks <wshaos@posteo.net>
chriseclectic
added
Changelog: API Change
* This PR adds the following consistency fixes for Calibrations: - check consistency between coupling map and control channel map - made date times as time zone - added qubits arg to update inst map - fixed issue with qubits in update_inst_map - Adds a methodology to deal with parameter updating in Calls for inst map. Co-authored-by: Will Shanks <wshaos@posteo.net>
3 participants
Summary
This is a fix for issue #558. The PR fixes several small bugs/inconsistencies in the
Calibrationsclass.Details and comments
The following issues/bugs/inconsistencies are fixed:
__init__of calibrations that forces users to specify a coupling map if thecontrol_channel_mapis given. The check requires that qubits in the keys ofcontrol_channel_mapare also contained as qubits in the coupling map. This is because the coupling map is used to determine the number of qubits of gate instructions (see_get_operated_qubits).update_inst_maphas the followingif qubits is not Nonechanged toif qubits. This change means that_robust_inst_map_addwill not run using default designated parameters (i.e.,qubits == tuple()) but will instead run over all schedules concerned by the change of the default parameter value. This is checked by the new testtest_inst_map_stays_consistent._get_channel_indexmethod has a small change so that a potentialKeyErrorwill not be raised until later in the code where the more meaningfulCalibrationErroris raised.ParameterValuenow makes sure that all date times have time-zone information.