Instruments section in runcard

[stavros11]

Fix #576. Needed for qibocal routines that update instrument (not qubit) parameters, such as the TWPA calibration. It is also useful to dump some instrument settings (LO frequency, etc.) in the runcard so that we can keep them in record. qibolab_platforms_qrc also needs to be updated for this to work.

@rodolfocarobene can you please confirm that my change in

qibolab/src/qibolab/instruments/rfsoc/convert.py

Line 39 in f896cc2

lo_frequency = getattr(qubits[pulse.qubit], pulse_type).local_oscillator.frequency

is okay?

Checklist:

• Reviewers confirm new code works as expected.
• Tests are passing.
• Coverage does not decrease.
• Documentation is updated.

[codecov]

Codecov Report

Patch coverage is 76.47% of modified lines.

Files Changed	Coverage
src/qibolab/instruments/rohde_schwarz.py	53.84%
src/qibolab/instruments/erasynth.py	61.53%
src/qibolab/instruments/oscillator.py	91.66%
src/qibolab/instruments/abstract.py	100.00%
src/qibolab/instruments/qm/simulator.py	100.00%
src/qibolab/instruments/rfsoc/convert.py	100.00%
src/qibolab/instruments/zhinst.py	100.00%
src/qibolab/serialize.py	100.00%

📢 Thoughts on this report? Let us know!.

[rodolfocarobene]

Hi @stavros11, I used the private property to avoid multiple connections to the local oscillators. I think that the problem is still there, right?

qibolab/src/qibolab/instruments/erasynth.py

Lines 30 to 36 in f896cc2

	@property
	def frequency(self):
	if self.is_connected:
	if self.ethernet:
	return int(self._get("frequency"))
	return self.device.get("frequency")
	return self.settings.frequency

[stavros11]

Hi @stavros11, I used the private property to avoid multiple connections to the local oscillators. I think that the problem is still there, right?

Good point, thanks @rodolfocarobene. I changed the R&S and ERA drivers to return the cached values, instead of getting from the instrument every time. This is simpler and should have the same behavior if the setter is implemented properly.

[andrea-pasquale]

Thanks @stavros11 for implementing this.
I did not test it yet but it looks good to me.
I will approve as soon as I test the PR.

[andrea-pasquale]

Thanks @stavros11. I tested it on hardware and it seems to work as expected.

[alecandido]

I like the minimal approach, and I see no problem in the implementation. Please, keep going :)

[alecandido]

@stavros11 are them really optional?

[stavros11]

@stavros11 are them really optional?

They are needed for execution. However, we initialize them as None so that we can instantiate the instrument without passing them in the platform create methods and assign their values later using instrument.setup (or the setters) after loading the runcard.

In principle we could initialize the instrument with self.settings = None and then instantiate settings when setup or the setters are called, but it is almost the same and maybe slightly more complicated in terms of implementation.

[alecandido]

On the one side, I'd agree with you for the current case. But if we had many more settings, the current one is scaling bad (you need linearly more code), while your second solution does not scale at all :)
(i.e. the second solution has arguably a constant overhead, or linearly less than the other one)