I also don't fully like this. In terms of this PR I did it to keep the behavior the same after the removal of platform.setup().
In general channel.offset can be different than qubit.sweetspot, for example if channel is not a flux channel or if we don't want to operate a qubit at its sweetspot. So it is not double source of truth, we are just assuming that we are operating all qubits at their sweetspots and we are setting the flux offset accordingly.
Other potential solutions I can think of are:
Drop offset from channels and modify drivers to use qubit.sweetspot as the offset of flux. This has many downsides as offset may be useful for non-flux channels and it will not be possible to operate qubits not at their sweetspot.
Drop qubit.sweetspot and just put the value on the offset directly as an instrument setting. Main downside of this is "political", people doing calibration may find it weird and the format of the runcards will change a bit so it should be communicated. Other than that that's not necessarily bad solution.
Keep both but the user has to set both manually. More flexible but will lead to repetition in the runcards, as the same value will be set twice in most cases. This is similar to the qubit frequency which is set as qubit.drive_frequency and also as the frequency of the native RX pulse.
Even @hay-k mentioned this to me, and he mentioned that also @aorgazf asked for this feature (that in practice consists in doing something less).
But I agree that for this PR might be better to keep the same behavior (just because the PR is about something else).
Solution 1. I don't like, because as you pointed out they could be two different objects, and because we don't want to repeat over and over some decision in all the drivers.
Every entity should be represented, and we should make proper use of them. If we want to change the offset, let's do it at higher level.
As you said yourself, solution 2. is better software-wise. But if we calibrate the sweetspot, it has to be recorded somewhere.
Solution 3. is close to what we have, and it's the one I'd keep. I'd simply drop the automated sync at this point.
We can have a mechanism (a method? a parameter in other method?) to do that (set the channel.offset out of the qubit.sweetspot) if it's a convenient shortcut for reading from the one side and setting to the other. Or otherwise do it manually. But the decision should always be explicit (unless we have an offset setting stage, and we just keep sweetspot as the default - but this stage should be easily accessible by the user).
Even @hay-k mentioned this to me, and he mentioned that also @aorgazf asked for this feature (that in practice consists in doing something less). But I agree that for this PR might be better to keep the same behavior (just because the PR is about something else).
Solution 1. I don't like, because as you pointed out they could be two different objects, and because we don't want to repeat over and over some decision in all the drivers. Every entity should be represented, and we should make proper use of them. If we want to change the offset, let's do it at higher level.
As you said yourself, solution 2. is better software-wise. But if we calibrate the sweetspot, it has to be recorded somewhere.
Solution 3. is close to what we have, and it's the one I'd keep. I'd simply drop the automated sync at this point. We can have a mechanism (a method? a parameter in other method?) to do that (set the
channel.offsetout of thequbit.sweetspot) if it's a convenient shortcut for reading from the one side and setting to the other. Or otherwise do it manually. But the decision should always be explicit (unless we have an offset setting stage, and we just keep sweetspot as the default - but this stage should be easily accessible by the user)._Originally posted by @AleCandido in https://github.com/qiboteam/qibolab/pull/739#discussion_r1447212076_