Remove characterization

[stavros11]

Fixes #945 by removing the characterization section of the runcard, since it is not needed for execution.

@alecandido this is probably undoing something that was done in #891, in particular I renamed the couplers back to 0, 1,... (instead of c0, c1,...). I think the only function that was causing an issue with name collisions between qubits and couplers was platform.elements, so I removed it, as I don't see the need for it. One can use platform.qubits and platform.couplers indepedently.

I also added back to the runcards and native gates the CP (coupler pulse). This is because since the "characterization" section no longer exists, we are now based on the native gates to register qubits and couplers. Therefore, if there are no native gates on couplers they will not be registered at all in the platform.

Tests are passing, however the change of threshold and angle needs to be propagated to the QM driver. I will probably do that together with the changes of #965 (I may even rebase this PR to that).

[codecov]

Codecov Report

Attention: Patch coverage is 98.41270% with 1 line in your changes missing coverage. Please review.

Project coverage is 42.90%. Comparing base (0d5b313) to head (af0d7f9).

Files	Patch %	Lines
src/qibolab/compilers/compiler.py	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              0.2     #972      +/-   ##
==========================================
- Coverage   43.57%   42.90%   -0.68%     
==========================================
  Files          80       80              
  Lines        5567     5505      -62     
==========================================
- Hits         2426     2362      -64     
- Misses       3141     3143       +2     

Flag	Coverage Δ
unittests	42.90% <98.41%> (-0.68%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[alecandido]

@alecandido this is probably undoing something that was done in #891, in particular I renamed the couplers back to 0, 1,... (instead of c0, c1,...). I think the only function that was causing an issue with name collisions between qubits and couplers was platform.elements, so I removed it, as I don't see the need for it. One can use platform.qubits and platform.couplers indepedently.

The name collision issue was not much caused by the platform.elements property, but I rather assumed that is convenient to be able to address an element even without its kind ahead of time.
I.e. if you are in a situation where you should apply a flux pulse on an element, given its ID, I see no reason not to prevent that (requiring further information about the element species).

E.g. I was using it in the compiler (of course in #965, sorry for the confusion):

qibolab/src/qibolab/compilers/compiler.py

Lines 142 to 162 in d61323e

	ch_to_qb = platform.channels_map
	sequence = PulseSequence()
	# FIXME: This will not work with qubits that have string names
	# TODO: Implement a mapping between circuit qubit ids and platform ``Qubit``s
	measurement_map = {}
	channel_clock = defaultdict(int)
	def qubit_clock(el: QubitId):
	return max(channel_clock[ch.name] for ch in platform.elements[el].channels)
	# process circuit gates
	for moment in circuit.queue.moments:
	for gate in set(filter(lambda x: x is not None, moment)):
	delay_sequence = PulseSequence()
	gate_sequence = self.get_sequence(gate, platform)
	increment = defaultdict(int)
	for ch in gate_sequence.channels:
	qubit = ch_to_qb[ch]
	delay = qubit_clock(qubit) - channel_clock[ch]

To be fair, I also don't like that much of having two dictionaries whose keys are implicitly concatenated, so, I see a few options:

1. we keep that distinction, and that's it (and we should solve the problem in the compiler somehow)
2. we mangle the names ourselves - in QubitId: drop str for internal usage #759 we will drop strings internally, and we could maintain the two species mapping them on positive and negative integers (or even and odd numbers)
3. we fully drop the distinction between qubits and couplers: we'll just have a single map of natives, including both qubits and couplers - the only difference will be in the channels registered in the platforms, since couplers won't have drive and readout registered, but just flux

Of course, I'm inclined towards 3., but open to discussion (especially, I'm not sure whether is possible, and if there is any drawback). We could even do in a further PR (e.g. #965 itself, where we have a use case for .elements)

Tests are passing, however the change of threshold and angle needs to be propagated to the QM driver. I will probably do that together with the changes of #965 (I may even rebase this PR to that).

To me, it's fine either way, but I'd appreciate rebasing #965 on top of this one or the opposite, since it's would be handy to have both to work on the serialization.
I could do the rebase.

[alecandido]

I also added back to the runcards and native gates the CP (coupler pulse). This is because since the "characterization" section no longer exists, we are now based on the native gates to register qubits and couplers. Therefore, if there are no native gates on couplers they will not be registered at all in the platform.

If that could be useful on its own, that's absolutely good.

However, if it's only duplicating the content of 2q gates, then I'd instead register an empty native. For the current layout, it is possible for SingleQubitNative to have only None attributes.

[stavros11]

Thanks for the quick feedback.

To me, it's fine either way, but I'd appreciate rebasing #965 on top of this one or the opposite, since it's would be handy to have both to work on the serialization. I could do the rebase.

I rebased this on top of #965 to unlock this. I did a quick patch for the compiler, but of course we can change this in the final version. This will even happen automatically by addressing some of your comments. I hope this is sufficient to not block your work, but I will try to address the rest of comments in the next days.

Actually this rebase is also convenient for me, as now I can fix both the sequence and the threshold/iq_angle/kernels in the QM driver together.

[stavros11]

1. we keep that distinction, and that's it (and we should solve the problem in the compiler somehow)
2. we mangle the names ourselves - in QubitId: drop str for internal usage #759 we will drop strings internally, and we could maintain the two species mapping them on positive and negative integers (or even and odd numbers)
3. we fully drop the distinction between qubits and couplers: we'll just have a single map of natives, including both qubits and couplers - the only difference will be in the channels registered in the platforms, since couplers won't have drive and readout registered, but just flux

Of course, I'm inclined towards 3., but open to discussion (especially, I'm not sure whether is possible, and if there is any drawback). We could even do in a further PR (e.g. #965 itself, where we have a use case for .elements)

Assuming 1, which is what is currently implemented here, the temporary patch for the compiler and elements is 543bfae. Personally, I am fine with 3, since it is less code for us, however one potential drawback is that qubits and couplers cannot have the same name which could be useful in some cases (such as iqm5q). Of course this is not an absolute deal breaker.

If that could be useful on its own, that's absolutely good.

However, if it's only duplicating the content of 2q gates, then I'd instead register an empty native. For the current layout, it is possible for SingleQubitNative to have only None attributes.

I also agree with this. I guess the answer to whether the "coupler native" is useful on its own depends on the routines that are used to calibrate it. I am guessing that it needs to be calibrated seperately from the 2q gates, but I am also not very familiar with the procedure. Maybe @andrea-pasquale or @Jacfomg know more.

[andrea-pasquale]

I also agree with this. I guess the answer to whether the "coupler native" is useful on its own depends on the routines that are used to calibrate it. I am guessing that it needs to be calibrated seperately from the 2q gates, but I am also not very familiar with the procedure. Maybe @andrea-pasquale or @Jacfomg know more.

I am not that expert on this but at least to my knowledge the only purpose of the coupler pulse is to turn on the interaction between two qubits. Therefore I am expecting that it shouldn't be needed anywhere else. If some particular applications require to send a pulse to the coupler we could always create one and assign it to coupler channel in runtime, correct?

[alecandido]

If some particular applications require to send a pulse to the coupler we could always create one and assign it to coupler channel in runtime, correct?

Creating arbitrary pulses will always be an option :)

[alecandido]

Ok, I rebased on present 0.2 (no conflict).

I already approved before, and it's fine by to merge as soon as you're ready.