TWPA calibration using SNR #664

DavidSarlle · 2023-12-08T08:05:45Z

This Pr implements 2 routines for characterizing the TWPA frequency and power doing a resonator spectroscopy while modifying the values of frequency / power of the TWPA.

Checklist:

Reviewers confirm new code works as expected.
Tests are passing.
Coverage does not decrease.
Documentation is updated.
Compatibility with Qibo modules (Please edit this section if the current pull request is not compatible with the following branches).
- Qibo: master
- Qibolab: main
- Qibolab_platforms_qrc: main

for more information, see https://pre-commit.ci

codecov · 2023-12-08T08:10:36Z

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (0e7a02b) 96.01% compared to head (fa86200) 96.00%.
Report is 17 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #664      +/-   ##
==========================================
- Coverage   96.01%   96.00%   -0.02%     
==========================================
  Files         106      108       +2     
  Lines        7356     7584     +228     
==========================================
+ Hits         7063     7281     +218     
- Misses        293      303      +10

Flag	Coverage Δ
unittests	`96.00% <95.61%> (-0.02%)`	⬇️

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

Files	Coverage Δ
src/qibocal/protocols/characterization/__init__.py	`100.00% <100.00%> (ø)`
...out_optimization/twpa_calibration/frequency_SNR.py	`95.53% <95.53%> (ø)`
...readout_optimization/twpa_calibration/power_SNR.py	`95.53% <95.53%> (ø)`

andrea-pasquale

Thanks @DavidSarlle.
The code looks more or less good to me.
The comments are mainly suggestions to have the code updated to the latest version.

Two important points:

It would be nice to perform a fitting, let me know if you need help with that.
Could you please add another entry for these new protocols here

src/qibocal/protocols/characterization/__init__.py

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

andrea-pasquale · 2023-12-08T08:15:59Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

+    resonator_type: str
+    """Resonator type."""


This attribute is not used anywhere

I will keep it because is going to be needed during the fitting.

andrea-pasquale · 2023-12-08T08:18:48Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

+    ro_pulses = {}
+    for qubit in qubits:
+        ro_pulses[qubit] = platform.create_qubit_readout_pulse(qubit, start=0)
+        sequence.add(ro_pulses[qubit])
+
+    data = ResonatorTWPAFrequencyData(platform.resonator_type)
+
+    # define the parameters to sweep and their range:
+    # resonator frequency
+    delta_frequency_range = np.arange(
+        -params.freq_width // 2, params.freq_width // 2, params.freq_step
+    )
+    freq_sweeper = Sweeper(
+        Parameter.frequency,
+        delta_frequency_range,
+        [ro_pulses[qubit] for qubit in qubits],
+        type=SweeperType.OFFSET,
+    )
+
+    # TWPAFrequency
+    TWPAFrequency_range = np.arange(
+        params.min_twpa_freq, params.max_twpa_freq, params.step_twpa_freq
+    )
+
+    for _freq in TWPAFrequency_range:
+        for z in qubits:
+            qubits[z].twpa.local_oscillator.frequency = _freq
+
+        results = platform.sweep(
+            sequence,
+            ExecutionParameters(
+                nshots=params.nshots,
+                relaxation_time=params.relaxation_time,
+                acquisition_type=AcquisitionType.INTEGRATION,
+                averaging_mode=AveragingMode.CYCLIC,
+            ),
+            freq_sweeper,
+        )


Since you are basically running a resonator spectroscopy varying the twpa you can just call directly the resonator spectroscopy protocol here. Like we did in the twpa optimization protocols by calling directly the classification protocol

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/power_SNR.py

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

for more information, see https://pre-commit.ci

…into david/twpa_SNR

for more information, see https://pre-commit.ci

andrea-pasquale

Thanks @DavidSarlle for all the updates.
I have a few more suggestions.
To fix coverage could you the two new protocols in this runcard ?

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

andrea-pasquale · 2024-01-11T05:56:24Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

+    for qubit in qubits:
+        data_qubit = data[qubit]
+        if data.resonator_type == "3D":
+            print("3D")


Suggested change

print("3D")

Remove all the print

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/power_SNR.py

andrea-pasquale · 2024-01-11T06:05:49Z

src/qibocal/protocols/characterization/resonator_spectroscopy.py

@@ -196,5 +196,5 @@ def _update(results: ResonatorSpectroscopyResults, platform: Platform, qubit: Qu
        update.bare_resonator_frequency(results.bare_frequency[qubit], platform, qubit)


-resonator_spectroscopy = Routine(_acquisition, _fit, _plot, _update)
+resonator_spec = Routine(_acquisition, _fit, _plot, _update)


Can I ask why you renamed resonator_spectroscopy in resonator_spec?

The name of the object "Routine" created in the in the "resonator_spectroscopy" class and the class itself have the same name. This is not a problem until you import that class from a different one. Python understands that you are importing the object "Routine" instead of the class. So, we can not use the dataclasses needed from the class "resonator_spectroscopy" like "ResonatorSpectroscopyParameters" during the twpa routines. Same with the private methods like "_acquisition" (but this is a minor problem because the Routine object inherits the methods needed)

DavidSarlle · 2024-01-12T06:39:00Z

Thanks @DavidSarlle for all the updates. I have a few more suggestions. To fix coverage could you the two new protocols in this runcard ?

Done

for more information, see https://pre-commit.ci

…into david/twpa_SNR

DavidSarlle · 2024-01-15T08:53:28Z

@andrea-pasquale the tests are not passing, can you help me with that?

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

Edoardo-Pedicillo · 2024-01-22T11:40:35Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

+    for qubit in qubits:
+        data_qubit = data[qubit]
+        if data.resonator_type == "3D":
+            print("3D")


Remove all the print

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

Edoardo-Pedicillo · 2024-01-22T11:46:12Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

+    if fit is not None:
+        if qubit in fit.bare_frequency:
+            summary = table_dict(
+                qubit,
+                [
+                    "High Power Resonator Frequency [Hz]",
+                    "TWPA Frequency [Hz]",
+                ],
+                [
+                    np.round(fit.bare_frequency[qubit]),
+                    np.round(fit.twpa_frequency[qubit]),
+                ],
+            )
+        else:
+            summary = table_dict(
+                qubit,
+                [
+                    "Low Power Resonator Frequency [Hz]",
+                    "TWPA Frequency [Hz]",
+                ],
+                [
+                    np.round(fit.frequency[qubit]),
+                    np.round(fit.twpa_frequency[qubit]),
+                ],
+            )
+


If possible, try not to repeat code, i.e. abstract all the common parts, or define a function collecting what inside the branches.

Edoardo-Pedicillo · 2024-01-22T12:04:50Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/power_SNR.py

+        size = len(freq)
+        ar = np.empty(size, dtype=ResonatorTWPAPowerType)
+        ar["freq"] = freq
+        ar["twpa_pow"] = np.array([twpa_pow] * size)


Add an attribute

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/power_SNR.py

Edoardo-Pedicillo · 2024-01-22T12:08:58Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

+
+    fitting_report = ""
+    qubit_data = data[qubit]
+    resonator_frequencies = qubit_data.freq * HZ_TO_GHZ


Suggested change

resonator_frequencies = qubit_data.freq * HZ_TO_GHZ

resonator_frequencies = qubit_data.freq

removed conversion

Here I don't understand why @Edoardo-Pedicillo asked you to remove the conversion. I suggest to perform the conversion mainly because I don't want to see bilions of Hz in the x axis. I already performed this change in fa86200

Sorry, I made a mistake

Edoardo-Pedicillo · 2024-01-22T12:10:23Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/power_SNR.py

+        if data.resonator_type == "3D":
+            index_best_pow = np.argmax(data_qubit["signal"])
+            twpa_power[qubit] = data_qubit["twpa_pow"][index_best_pow]
+        else:
+            index_best_pow = np.argmin(data_qubit["signal"])
+            twpa_power[qubit] = data_qubit["twpa_pow"][index_best_pow]


Suggested change

if data.resonator_type == "3D":

index_best_pow = np.argmax(data_qubit["signal"])

twpa_power[qubit] = data_qubit["twpa_pow"][index_best_pow]

else:

index_best_pow = np.argmin(data_qubit["signal"])

twpa_power[qubit] = data_qubit["twpa_pow"][index_best_pow]

if data.resonator_type == "3D":

index_best_pow = np.argmax(data_qubit["signal"])

else:

index_best_pow = np.argmin(data_qubit["signal"])

twpa_power[qubit] = data_qubit["twpa_pow"][index_best_pow]

Implemented

Edoardo-Pedicillo · 2024-01-22T12:17:53Z

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py

+    def register_qubit(self, qubit, freq, twpa_freq, signal, phase):
+        """Store output for single qubit."""
+        size = len(freq)
+        ar = np.empty(size, dtype=ResonatorTWPAFrequencyType)
+        ar["freq"] = freq
+        ar["twpa_freq"] = np.array([twpa_freq] * size)
+        ar["signal"] = signal
+        ar["phase"] = phase
+        if qubit in self.data:
+            self.data[qubit] = np.rec.array(np.concatenate((self.data[qubit], ar)))
+        else:
+            self.data[qubit] = np.rec.array(ar)


Suggested change

def register_qubit(self, qubit, freq, twpa_freq, signal, phase):

"""Store output for single qubit."""

size = len(freq)

ar = np.empty(size, dtype=ResonatorTWPAFrequencyType)

ar["freq"] = freq

ar["twpa_freq"] = np.array([twpa_freq] * size)

ar["signal"] = signal

ar["phase"] = phase

if qubit in self.data:

self.data[qubit] = np.rec.array(np.concatenate((self.data[qubit], ar)))

else:

self.data[qubit] = np.rec.array(ar)

The class Data has a default register_qubit method, and it seems that it is not different from the one you implemented.

qibocal/src/qibocal/auto/operation.py

Lines 164 to 181 in 1adce17

def register_qubit(self, dtype, data_keys, data_dict):

"""Store output for single qubit.

Args:

data_keys (tuple): Keys of Data.data.

data_dict (dict): The keys are the fields of `dtype` and

the values are the related arrays.

"""

# to be able to handle the non-sweeper case

ar = np.empty(np.shape(data_dict[list(data_dict.keys())[0]]), dtype=dtype)

for key, value in data_dict.items():

ar[key] = value

if data_keys in self.data:

self.data[data_keys] = np.rec.array(

np.concatenate((self.data[data_keys], ar))

)

else:

self.data[data_keys] = np.rec.array(ar)

Thanks. Implemented

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/power_SNR.py

for more information, see https://pre-commit.ci

…into david/twpa_SNR

for more information, see https://pre-commit.ci

DavidSarlle · 2024-01-24T07:25:10Z

@Edoardo-Pedicillo all your comments has been addressed. Thanks for the review.

andrea-pasquale

Thanks for all the updates @DavidSarlle.
I believe we are ready to merge.
Just before merging...
Regarding this:

The name of the object "Routine" created in the in the "resonator_spectroscopy" class and the class itself have the same name. This is not a problem until you import that class from a different one. Python understands that you are importing the object "Routine" instead of the class. So, we can not use the dataclasses needed from the class "resonator_spectroscopy" like "ResonatorSpectroscopyParameters" during the twpa routines. Same with the private methods like "_acquisition" (but this is a minor problem because the Routine object inherits the methods needed)
I tried to replace resonator_spec with resonator_spectroscopy and everything worked as expected. If you want I can push the change directly here.

DavidSarlle · 2024-01-24T08:31:18Z

Thanks for all the updates @DavidSarlle. I believe we are ready to merge. Just before merging... Regarding this:

The name of the object "Routine" created in the in the "resonator_spectroscopy" class and the class itself have the same name. This is not a problem until you import that class from a different one. Python understands that you are importing the object "Routine" instead of the class. So, we can not use the dataclasses needed from the class "resonator_spectroscopy" like "ResonatorSpectroscopyParameters" during the twpa routines. Same with the private methods like "_acquisition" (but this is a minor problem because the Routine object inherits the methods needed)
I tried to replace resonator_spec with resonator_spectroscopy and everything worked as expected. If you want I can push the change directly here.

ok. push the change please.
Thanks!

andrea-pasquale · 2024-01-24T08:52:22Z

Thanks for all the updates @DavidSarlle. I believe we are ready to merge. Just before merging... Regarding this:

The name of the object "Routine" created in the in the "resonator_spectroscopy" class and the class itself have the same name. This is not a problem until you import that class from a different one. Python understands that you are importing the object "Routine" instead of the class. So, we can not use the dataclasses needed from the class "resonator_spectroscopy" like "ResonatorSpectroscopyParameters" during the twpa routines. Same with the private methods like "_acquisition" (but this is a minor problem because the Routine object inherits the methods needed)
I tried to replace resonator_spec with resonator_spectroscopy and everything worked as expected. If you want I can push the change directly here.

ok. push the change please. Thanks!

Actually it was not as easy as I expected. Strangely only pylint complains but not pytest. I've also tested the code and it works as expected. I manage to fix it but by importing directly the resonator_spectroscopy routine object. In this way pylint does not complain.

DavidSarlle and others added 2 commits December 8, 2023 12:03

implementing twpa calibration using SNR as a figuere of merit

a345e28

[pre-commit.ci] auto fixes from pre-commit.com hooks

162ffff

for more information, see https://pre-commit.ci

DavidSarlle changed the title ~~implementing twpa calibration using SNR as a figuere of merit~~ TWPA calibration using SNR Dec 8, 2023

DavidSarlle requested review from andrea-pasquale and Edoardo-Pedicillo December 8, 2023 08:06

andrea-pasquale requested changes Dec 8, 2023

View reviewed changes

andrea-pasquale reviewed Dec 8, 2023

View reviewed changes

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py Outdated Show resolved Hide resolved

DavidSarlle and others added 7 commits January 10, 2024 11:13

reusing resonator spectroscopy to perform twpa calibration freq

8d46943

[pre-commit.ci] auto fixes from pre-commit.com hooks

3f131d9

for more information, see https://pre-commit.ci

adddressing all review comments

e7c40fa

Merge branch 'david/twpa_SNR' of https://github.com/qiboteam/qibocal …

3114022

…into david/twpa_SNR

removing unused imports

9414764

[pre-commit.ci] auto fixes from pre-commit.com hooks

885ce83

for more information, see https://pre-commit.ci

Merge branch 'main' into david/twpa_SNR

31bef10

andrea-pasquale reviewed Jan 11, 2024

View reviewed changes

andrea-pasquale added the calibration label Jan 12, 2024

DavidSarlle and others added 6 commits January 13, 2024 00:12

addressing latest comments

f5a7c53

[pre-commit.ci] auto fixes from pre-commit.com hooks

31d0634

for more information, see https://pre-commit.ci

testing modifications

265fb67

Merge branch 'david/twpa_SNR' of https://github.com/qiboteam/qibocal …

76ace75

…into david/twpa_SNR

tests

12820d3

test

f4ff7da

andrea-pasquale reviewed Jan 15, 2024

View reviewed changes

src/qibocal/protocols/characterization/readout_optimization/twpa_calibration/frequency_SNR.py Outdated Show resolved Hide resolved

fixing tests

1adce17

Edoardo-Pedicillo requested changes Jan 22, 2024

View reviewed changes

DavidSarlle and others added 2 commits January 22, 2024 20:55

adressing Edoardo's review comments

29f3b07

[pre-commit.ci] auto fixes from pre-commit.com hooks

40e1427

for more information, see https://pre-commit.ci

DavidSarlle and others added 5 commits January 22, 2024 20:56

removing impor not used

4f631df

Merge branch 'david/twpa_SNR' of https://github.com/qiboteam/qibocal …

e8abb9c

…into david/twpa_SNR

[pre-commit.ci] auto fixes from pre-commit.com hooks

74026df

for more information, see https://pre-commit.ci

addressing rest of Edoardo's comments

e3a2843

[pre-commit.ci] auto fixes from pre-commit.com hooks

de8cf42

for more information, see https://pre-commit.ci

andrea-pasquale approved these changes Jan 24, 2024

View reviewed changes

refactor: Rename resonator_spec -> resonator_spectroscopy

2ddacbe

refactor: merge main and improve plots

fa86200

andrea-pasquale requested a review from Edoardo-Pedicillo January 24, 2024 09:16

andrea-pasquale enabled auto-merge January 24, 2024 09:55

Edoardo-Pedicillo approved these changes Feb 1, 2024

View reviewed changes

andrea-pasquale added this pull request to the merge queue Feb 1, 2024

Merged via the queue into main with commit a5217a3 Feb 1, 2024
20 of 21 checks passed

andrea-pasquale deleted the david/twpa_SNR branch February 1, 2024 12:20

andrea-pasquale mentioned this pull request Feb 1, 2024

Resonator spec vs TWPA freq/power and fit the best SNR point #600

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TWPA calibration using SNR #664

TWPA calibration using SNR #664

DavidSarlle commented Dec 8, 2023 •

edited

Loading

codecov bot commented Dec 8, 2023 •

edited

Loading

andrea-pasquale left a comment

andrea-pasquale Dec 8, 2023

DavidSarlle Jan 9, 2024

andrea-pasquale Dec 8, 2023

DavidSarlle Jan 10, 2024

andrea-pasquale left a comment

andrea-pasquale Jan 11, 2024

DavidSarlle Jan 12, 2024

Edoardo-Pedicillo Jan 22, 2024

andrea-pasquale Jan 11, 2024

DavidSarlle Jan 12, 2024 •

edited

Loading

DavidSarlle commented Jan 12, 2024

DavidSarlle commented Jan 15, 2024

Edoardo-Pedicillo Jan 22, 2024

Edoardo-Pedicillo Jan 22, 2024

DavidSarlle Jan 22, 2024

Edoardo-Pedicillo Jan 22, 2024

Edoardo-Pedicillo Jan 22, 2024

DavidSarlle Jan 22, 2024

andrea-pasquale Jan 24, 2024

Edoardo-Pedicillo Feb 1, 2024

Edoardo-Pedicillo Jan 22, 2024

DavidSarlle Jan 22, 2024

Edoardo-Pedicillo Jan 22, 2024

DavidSarlle Jan 24, 2024

DavidSarlle commented Jan 24, 2024

andrea-pasquale left a comment

DavidSarlle commented Jan 24, 2024

andrea-pasquale commented Jan 24, 2024

	resonator_frequencies = qubit_data.freq * HZ_TO_GHZ
	resonator_frequencies = qubit_data.freq

	def register_qubit(self, dtype, data_keys, data_dict):
	"""Store output for single qubit.

	Args:
	data_keys (tuple): Keys of Data.data.
	data_dict (dict): The keys are the fields of `dtype` and
	the values are the related arrays.
	"""
	# to be able to handle the non-sweeper case
	ar = np.empty(np.shape(data_dict[list(data_dict.keys())[0]]), dtype=dtype)
	for key, value in data_dict.items():
	ar[key] = value
	if data_keys in self.data:
	self.data[data_keys] = np.rec.array(
	np.concatenate((self.data[data_keys], ar))
	)
	else:
	self.data[data_keys] = np.rec.array(ar)

TWPA calibration using SNR #664

TWPA calibration using SNR #664

Conversation

DavidSarlle commented Dec 8, 2023 • edited Loading

codecov bot commented Dec 8, 2023 • edited Loading

Codecov Report

andrea-pasquale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrea-pasquale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidSarlle Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

DavidSarlle commented Jan 12, 2024

DavidSarlle commented Jan 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidSarlle commented Jan 24, 2024

andrea-pasquale left a comment

Choose a reason for hiding this comment

DavidSarlle commented Jan 24, 2024

andrea-pasquale commented Jan 24, 2024

DavidSarlle commented Dec 8, 2023 •

edited

Loading

codecov bot commented Dec 8, 2023 •

edited

Loading

DavidSarlle Jan 12, 2024 •

edited

Loading