Skip to content

spin lock sequence - #1540

Draft
jevillegasd wants to merge 12 commits into
mainfrom
spin_lock_spectroscopy
Draft

spin lock sequence#1540
jevillegasd wants to merge 12 commits into
mainfrom
spin_lock_spectroscopy

Conversation

@jevillegasd

Copy link
Copy Markdown
Contributor

This is a first trial to address #1525

The experiment runs, but I run out of memory surprisingly fast, even though I use sweepers for both amplitude and time iterators. Maybe this works better if I use of of the iterations to be a for loop outside (at the cost of the experiment being longer than necessary).

@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.57143% with 111 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.40%. Comparing base (e0d5dcc) to head (b00f190).

Files with missing lines Patch % Lines
src/qibocal/protocols/coherence/spin_lock.py 36.20% 111 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1540      +/-   ##
==========================================
- Coverage   94.36%   93.40%   -0.97%     
==========================================
  Files         136      137       +1     
  Lines       10634    10809     +175     
==========================================
+ Hits        10035    10096      +61     
- Misses        599      713     +114     
Flag Coverage Δ
unittests 93.40% <36.57%> (-0.97%) ⬇️

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

Files with missing lines Coverage Δ
src/qibocal/protocols/coherence/__init__.py 100.00% <100.00%> (ø)
src/qibocal/protocols/coherence/spin_lock.py 36.20% <36.20%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jevillegasd

Copy link
Copy Markdown
Contributor Author

Im kind of stuck trying to make the protocol run for long durations of the spin lock pulse. I see that the driver supports batching , would this be a good way to allow longer sequences.

@alecandido

Copy link
Copy Markdown
Member

Im kind of stuck trying to make the protocol run for long durations of the spin lock pulse. I see that the driver supports batching , would this be a good way to allow longer sequences.

Just for the sake of future reference: batching is quite the opposite. It is a way to accelerate the execution of short sequences, but quite ineffective on long ones.

For very long sequences, we just need to identify the limiting factor (instruction memory, waveform memory, acquisition memory) and check whether there is a specific implementation lifting the bottleneck.

In this case, the sequence is in a sense small, since it is made by few operations. The reason it is failing is because of waveform memory limitations, which can only be lifted by not realizing the spin-locking pulse as a single arbitrary waveform. Which can be realized by either decomposing it in parts (e.g. repeating many back-to-back identical rectangular units) or with a compressed pulse implementation (which is possible for the flux pulse).

@lballerio lballerio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think is good, just small suggestions.
Btw @jevillegasd can you also paste on this PR some data plot you generated while running this experiment?

Comment on lines +55 to +60
duration_min: int
"""Minimum spin-lock pulse duration [ns]."""
duration_max: int
"""Maximum spin-lock pulse duration [ns]."""
duration_step: int
"""Step spin-lock pulse duration [ns]."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we decided to move into ranges as input, i.e. tuples of the form (init_value, final_value, step).

Suggested change
duration_min: int
"""Minimum spin-lock pulse duration [ns]."""
duration_max: int
"""Maximum spin-lock pulse duration [ns]."""
duration_step: int
"""Step spin-lock pulse duration [ns]."""
duration_range: tuple[float, float, float]
"""Spin-lock pulse duration range [ns]."""

Also in theory we can admit times to be float for sub-ns sampling rate; then drivers will convert it accordingly.

Comment on lines +70 to +73
@property
def duration_range(self) -> tuple[int, int, int]:
"""Return a tuple with the spin-lock pulse duration range."""
return self.duration_min, self.duration_max, self.duration_step

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noy necessary anymore since we are taking range as input.

Suggested change
@property
def duration_range(self) -> tuple[int, int, int]:
"""Return a tuple with the spin-lock pulse duration range."""
return self.duration_min, self.duration_max, self.duration_step

Comment on lines +61 to +66
amplitude_min: float
"""Minimum spin-lock pulse amplitude [a.u.]."""
amplitude_max: float
"""Maximum spin-lock pulse amplitude [a.u.]."""
n_amplitudes: int
"""Number of amplitude points, log-spaced between amplitude_min and amplitude_max."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly here I don't know whether is better to ask for a tuple or not. However I will write amplitude_values as a @property of the SpinLockParameters class.

Comment on lines +82 to +85
gamma: dict[QubitId, list[float]] = field(default_factory=dict)
"""Relaxation rate :math:`\\Gamma_{1\\rho} = S(\\nu_R)` for each amplitude [Hz]."""
gamma_error: dict[QubitId, list[float]] = field(default_factory=dict)
"""Error on :math:`\\Gamma_{1\\rho}` [Hz]."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to save them together?
Something like:

Suggested change
gamma: dict[QubitId, list[float]] = field(default_factory=dict)
"""Relaxation rate :math:`\\Gamma_{1\\rho} = S(\\nu_R)` for each amplitude [Hz]."""
gamma_error: dict[QubitId, list[float]] = field(default_factory=dict)
"""Error on :math:`\\Gamma_{1\\rho}` [Hz]."""
gamma: dict[QubitId, list[float, float]] = field(default_factory=dict)
"""Relaxation rate :math:`\\Gamma_{1\\rho} = S(\\nu_R)` and its error for each amplitude [Hz]."""

) -> SpinLockData:
"""Data acquisition for the spin-lock (T1rho) experiment."""

duration_range = np.arange(*params.duration_range)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call in a different way since it is not a range and then might be confused with params.duration_range.

Comment on lines +162 to +164
qd_channel = platform.qubits[q].drive
ro_channel, ro_pulse = natives.MZ()[0]
rx_pulse = natives.RX()[0][1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why not:

ro_channel, ro_pulse = natives.MZ()[0]
rx_pulse, qd_channel = natives.RX()[0][1]

that I cannot see?

nshots=params.nshots,
relaxation_time=params.relaxation_time,
acquisition_type=AcquisitionType.DISCRIMINATION,
averaging_mode=AveragingMode.SINGLESHOT,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you set SINGLESHOT as acquisition mode and not CYCLIC?

)

for q in targets:
prob = probability(results[ro_pulses[q].id], state=1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you use CYCLIC:

Suggested change
prob = probability(results[ro_pulses[q].id], state=1)
prob = results[ro_pulses[q].id]

Comment on lines +261 to +262
gamma = {}
gamma_error = {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as before, maybe error and measurement can be incorporated

return SpinLockResults(t1rho, gamma, gamma_error, rabi_frequency, fitted_parameters)


def _plot(data: SpinLockData, target: QubitId, fit: SpinLockResults = None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def _plot(data: SpinLockData, target: QubitId, fit: SpinLockResults = None):
def _plot(data: SpinLockData, target: QubitId, fit: SpinLockResults | None = None):

@lballerio

Copy link
Copy Markdown
Contributor

Btw also rebase (in this case is straightforward) and try to solve the failing tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants