Serialization interface

[alecandido]

Closes #946

• investigate arbitrary_types_allowed, cf. 6105cb3
• replace load_instrument_settings with components
• automate loading of Runcard into platform
• automate components loading
• add methods for template creation cf. Parameters templates and handling #978
• add kernels back into components
• check whether it's possible to separate runcard configs from those defined in platform.py Separate channels from natives #982
• fix 922d49d

---

Just in case anyone will look at this later on: it definitely went beyond the interface.

[codecov]

Codecov Report

Attention: Patch coverage is 95.31773% with 14 lines in your changes missing coverage. Please review.

Project coverage is 41.71%. Comparing base (5d94231) to head (caa8da7).
Report is 73 commits behind head on 0.2.

Files	Patch %	Lines
src/qibolab/compilers/compiler.py	88.00%	3 Missing ⚠️
src/qibolab/qubits.py	72.72%	3 Missing ⚠️
src/qibolab/native.py	93.33%	2 Missing ⚠️
src/qibolab/platform/platform.py	95.83%	2 Missing ⚠️
src/qibolab/instruments/abstract.py	0.00%	1 Missing ⚠️
...rc/qibolab/instruments/emulator/pulse_simulator.py	0.00%	1 Missing ⚠️
src/qibolab/parameters.py	98.14%	1 Missing ⚠️
src/qibolab/serialize.py	97.05%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              0.2     #971      +/-   ##
==========================================
- Coverage   42.93%   41.71%   -1.23%     
==========================================
  Files          80       79       -1     
  Lines        5524     5442      -82     
==========================================
- Hits         2372     2270     -102     
- Misses       3152     3172      +20     

Flag	Coverage Δ
unittests	41.71% <95.31%> (-1.23%)	⬇️

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]

@stavros11 I now introduced the Runcard object, that should be an exact mirror of the content of the runcard dumped.

It seems reasonable to me to have this object in the Platform, instead of translating its attributes from one side to the other.
The plan is that Runcard will contain everything that is automatically serializable (in particular the parameters subject to changes, because of calibration). While Platform will contain a Runcard instance, and all the information that is only provided in the .py file on top (in particular, the instruments and the connections).

What do you think about this plan?

P.S.: I'd like to have a better name than Runcard, but I'm missing one...
(the only thing that comes to my mind is Parameters, which is the same of parameters.json, and would reduce the confusion with Qibocal's runcards, but it's also a bit meh)

[stavros11]

It seems reasonable to me to have this object in the Platform, instead of translating its attributes from one side to the other. The plan is that Runcard will contain everything that is automatically serializable (in particular the parameters subject to changes, because of calibration). While Platform will contain a Runcard instance, and all the information that is only provided in the .py file on top (in particular, the instruments and the connections).

What do you think about this plan?

Thanks for the summary. Sure, sounds good to me as well, as long as we are not duplicating the information (eg. copying from the Runcard to Platform) and instead the Platform just queries the Runcard. After a quick look it seems that this is what you are doing, so I agree.

P.S.: I'd like to have a better name than Runcard, but I'm missing one... (the only thing that comes to my mind is Parameters, which is the same of parameters.json, and would reduce the confusion with Qibocal's runcards, but it's also a bit meh)

Runcard is the straightforward choice based on history, since that's how everyone is calling that. I agree that it is not the best here and is better suited to qibocal that is actually being used to specify what to "run" (in contrast to here). Parameters is not that bad, probably better than Runcard. One alternative that I can think of now is Calibration.

[alecandido]

I agree that it is not the best here and is better suited to qibocal that is actually being used to specify what to "run" (in contrast to here). Parameters is not that bad, probably better than Runcard.

Let's try.

One alternative that I can think of now is Calibration.

I also thought about something like that, but I felt guilty wrt #972 :P

[alecandido]

@stavros11 as you'll observe, there is a bit of additional mess in the sequence class:

qibolab/src/qibolab/pulses/sequence.py

Lines 19 to 48 in 42572a8

	class PulseSequence(UserList[_Element]):
	"""Synchronized sequence of control instructions across multiple channels.
	The sequence is a linear stream of instructions, which may be
	executed in parallel over multiple channels.
	Each instruction is composed by the pulse-like object representing
	the action, and the channel on which it should be performed.
	"""
	@classmethod
	def __get_pydantic_core_schema__(
	cls, source_type: Any, handler: Callable[[Any], core_schema.CoreSchema]
	) -> core_schema.CoreSchema:
	schema = handler(list[_Element])
	return core_schema.no_info_after_validator_function(
	cls._validate,
	schema,
	serialization=core_schema.plain_serializer_function_ser_schema(
	cls._serialize, info_arg=False, return_schema=schema
	),
	)
	@classmethod
	def _validate(cls, value):
	return cls(value)
	@staticmethod
	def _serialize(value):
	return TypeAdapter(list[_Element]).dump_python(list(value))

I just wanted to tell you in advance that, despite working, this is only needed because I'm trying to keep PulseSequence a subclass of list.
If instead we decided to have PulseSequence as a dataclass (now a model) with a single field, there would be no problem instead (but then we may have to alias all the relevant list methods again, or to always access the internal attribute for the actual sequence).

This is a trade-off, and I'm not fully satisfied myself. If things will change, we may switch decision (e.g. keep it simple, and alias some methods manually, like __getitem__ and __iter__).

[alecandido]

Another annoying point is about the qubits, since they contain two things, with two different sources:

• the natives, which come from the parameters.json
• and the channels, which come from the platform.py

This leads to this mess:

qibolab/src/qibolab/platform/platform.py

Lines 118 to 161 in 2f1683f

	@property
	def qubits(self) -> QubitMap:
	"""Mapping qubit names to :class:`qibolab.qubits.Qubit` objects."""
	if len(self._qubits) == 0:
	for q, natives in self.parameters.native_gates.single_qubit.items():
	self._qubits[q] = Qubit(name=q, native_gates=natives)
	for q, natives in self.parameters.native_gates.single_qubit.items():
	self._qubits[q].native_gates = natives
	return self._qubits
	@property
	def couplers(self) -> QubitMap:
	"""Mapping coupler names to :class:`qibolab.qubits.Qubit` objects."""
	if len(self._couplers) == 0:
	for c, natives in self.parameters.native_gates.coupler.items():
	self._couplers[c] = Qubit(name=c, native_gates=natives)
	for c, natives in self.parameters.native_gates.coupler.items():
	self._couplers[c].native_gates = natives
	return self._couplers
	@property
	def pairs(self) -> QubitPairMap:
	"""Mapping tuples of qubit names to :class:`qibolab.qubits.QubitPair`
	objects."""
	if len(self._pairs) == 0:
	for p, natives in self.parameters.native_gates.two_qubit.items():
	self._pairs[p] = QubitPair(
	qubit1=p[0], qubit2=p[1], native_gates=natives
	)
	if natives.symmetric:
	self._pairs[(p[1], p[0])] = QubitPair(
	qubit1=p[0], qubit2=p[1], native_gates=natives
	)
	for p, natives in self.parameters.native_gates.two_qubit.items():
	self._pairs[p].native_gates = natives
	if natives.symmetric:
	self._pairs[(p[1], p[0])].native_gates = natives
	return self._pairs

Is it acceptable to give up on the Qubit object to contain the natives? Can we imagine a different interface to access them?
(similarly to what is happening for the channels, which do not contain their parameters, but just the reference as a name)

[alecandido]

• investigate arbitrary_types_allowed, cf. 6105cb3

I actually forgot it, but arbitrary_types_allowed is required to use some nested classes that are neither basic types nor Model subclasses (or just Pydantic compatible).
An example is NdArray, that is also the motivation I introduced it. And maybe that is the only example.

[alecandido]

@stavros11 I still want to check whether I can elegantly split the channels from their configs, and to remove the cumbersome qubit/coupler/pair properties and attributes (and I have one Zurich doctest to fix).

However, the PR is already pretty big, and the main features are implemented.
Thus, I'd encourage you to take a look, and to check whether you have some further proposals for the interface.
Notice that this PR is about the "Serialization interface", but the only true interface is the one involved in the platform creation, where there is practically nothing left about serialization (other than Platform.load()).

[alecandido]

3cdf01b should not be needed at all, is somewhat like a Pydantic bug (or a bad interaction with Sphinx dynamic imports). It could not happen as a standalone script.

However, this particular instance is happening because Zurich is not reached by the bare import qibolab (correct, otherwise its dependencies would not be optional), and triggered by subclassing a model subclass (well, Model itself is a subclass of BaseModel, but the main difference is the one above: they were already reached by import qibolab - no idea why this is relevant, at most hypotheses).

Anyhow, this could also be solved by avoiding driver-specific components.
It's not a good reason on its own to do that, since it's a bug (even a nasty combined one) - for which there are good reasons. I'm only reporting it could lift the need for this workaround.

[alecandido]

The last item remaining was big enough to be worth a separate PR, even for the advantage of the review.

This is now ready as it is, and it could be used to upgrade QM and its platforms.

(the remaining changes won't affect that much the platform-creation interface, which is the true name of the serialization interface mentioned in this PR's title)

[stavros11]

I have the impression that Qubit can be frozen. If not in this PR, since it still contains the native gates which may need to be updated (eg. by qibocal) then in #983. It only contains the logical channels (not their configs), which I believe never change.

[alecandido]

In principle, it could: already in #983 it should be created in the platform, and never touched again...

Almost. For as long as the frequencies are in the channels, we need to leave Qibocal (and any user) the freedom to change that even during the execution (i.e. between two of them).

[stavros11]

Almost. For as long as the frequencies are in the channels, we need to leave Qibocal (and any user) the freedom to change that even during the execution (i.e. between two of them).

I think the frequencies are in the configs, not the channels. The configs are inheriting the default Model from qibolab.serialize which I believe is frozen(?). Therefore, the user already cannot change the frequency directly but they can

1. pass a different config when executing something or,
2. change the default config in the Platform

which I believe are acceptable solutions.

The Qubit seems to be frozen anyway now. It only has a name (frozen) and channels and the channels only have str attributes, all of which I believe are frozen. That's why I think we don't need to "unfreeze" it.

While looking on that, I realized that the Qubit also has a mixer_frequencies property, which I believe is broken as it tries to access attributes that don't exist anymore (not confirmed though, as I have not tried to use it). We should probably drop this.

[alecandido]

I think the frequencies are in the configs, not the channels.

Correct

The configs are inheriting the default Model from qibolab.serialize which I believe is frozen(?).

Also correct

which I believe are acceptable solutions.

The problem is what you would if you need more frequencies. In that case, you will have to add new channels.

The Qubit seems to be frozen anyway now. It only has a name (frozen) and channels and the channels only have str attributes, all of which I believe are frozen. That's why I think we don't need to "unfreeze" it.

All correct. But one of the channels is actually a dictionary (cross resonances), which is a mutable object (opposite to strings).
In principle, this is not preventing Qubit from being frozen (which was only needed for natives), but it would be a bit fake: the address of the dictionary won't change, and that's going to make it possible to freeze the object. But the dictionary items would be conceptually part of the Qubit, and they can change at will.

We can choose what we prefer. Freeze or unfreeze shouldn't matter that much in this case.

While looking on that, I realized that the Qubit also has a mixer_frequencies property, which I believe is broken as it tries to access attributes that don't exist anymore (not confirmed though, as I have not tried to use it). We should probably drop this.

Agreed.

[stavros11]

All correct. But one of the channels is actually a dictionary (cross resonances), which is a mutable object (opposite to strings). In principle, this is not preventing Qubit from being frozen (which was only needed for natives), but it would be a bit fake: the address of the dictionary won't change, and that's going to make it possible to freeze the object. But the dictionary items would be conceptually part of the Qubit, and they can change at will.

You are right, I missed this dict. In that case, I am also fine with leaving it as not frozen.

However, I have the impression that allowing users to attach new channels to qubits is not very useful. To execute pulses on these channels they would also have to create the corresponding instrument channel to provide the port information and also register this to the instrument. QM, which is the only working driver, does not provide any interface for this. Of course it's Python so you can still access the QmController from the Platform and attach the channel there. This would work, however it requires so much knowledge of the internals that I wouldn't call you a "user" anymore.

Not for this PR, but regarding the channel creation business, we should probably decide: we either drop completely and in that case I think we can safely freeze Qubit, or we provide a more user-friendly way to do it. Even if we drop, users could still attach new channels in the platform creation if needed, during Qubit creation, so that there is no need to mutate them.

[stavros11]

I think the things I would like to have fixed before merging this are the helper property to access native gates and potentially make the Qubit frozen and drop mixer_frequencies, since this is very simple. Everything else I would leave for the future and may even be debatable.

[stavros11]

Almost. For as long as the frequencies are in the channels, we need to leave Qibocal (and any user) the freedom to change that even during the execution (i.e. between two of them).

I think the frequencies are in the configs, not the channels. The configs are inheriting the default Model from qibolab.serialize which I believe is frozen(?). Therefore, the user already cannot change the frequency directly but they can

1. pass a different config when executing something or,
2. change the default config in the Platform

which I believe are acceptable solutions.

The Qubit seems to be frozen anyway now. It only has a name (frozen) and channels and the channels only have str attributes, all of which I believe are frozen. That's why I think we don't need to "unfreeze" it.

While looking on that, I realized that the Qubit also has a mixer_frequencies property, which I believe is broken as it tries to access attributes that don't exist anymore (not confirmed though, as I have not tried to use it). We should probably drop this.

[alecandido]

Ok, all the comments should be addressed, but the last two still open (which are not for this PR, in principle).

I could provide some more tests, but I'd prefer to finish first all the other features, and then review the tests altogether (there are choices to be made and simplifications to be applied).

[stavros11]

Thanks for the updates @alecandido. I believe we can now merge this, right? It was also tested and working with the QM driver (using #989).