Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flux-Drive timing #1056

Open
wants to merge 16 commits into
base: 0.2
Choose a base branch
from
Open

Flux-Drive timing #1056

wants to merge 16 commits into from

Conversation

Edoardo-Pedicillo
Copy link
Contributor

@Edoardo-Pedicillo Edoardo-Pedicillo commented Dec 9, 2024

This PR implements the protocol XY-Z timing (https://escholarship.org/content/qt0g29b4p0/qt0g29b4p0.pdf?t=prk0gj par. 5.10).

http://login.qrccluster.com:9000/xWr3LZnfTQmN59r2aFetnA==/

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

Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.01%. Comparing base (9723091) to head (f759ac6).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              0.2    #1056      +/-   ##
==========================================
+ Coverage   96.96%   97.01%   +0.05%     
==========================================
  Files          98       99       +1     
  Lines        7932     8042     +110     
==========================================
+ Hits         7691     7802     +111     
+ Misses        241      240       -1     
Flag Coverage Δ
unittests 97.01% <100.00%> (+0.05%) ⬆️

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

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

... and 3 files with indirect coverage changes

@Edoardo-Pedicillo Edoardo-Pedicillo marked this pull request as draft December 9, 2024 06:50
@Edoardo-Pedicillo Edoardo-Pedicillo marked this pull request as ready for review January 20, 2025 12:12
Copy link
Contributor

@andrea-pasquale andrea-pasquale left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation @Edoardo-Pedicillo.
I have a few questions in the comments below. Apart from a problem with the for loop the code runs fine.

doc/source/protocols/xyz_timing.rst Outdated Show resolved Hide resolved
doc/source/protocols/xyz_timing.rst Outdated Show resolved Hide resolved

This routine is evaluating the different time of arrival of the flux and drive
pulse to the qubit. These delays are usually originated by the differences in
cable length or by the electronics.
Copy link
Contributor

Choose a reason for hiding this comment

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

With "by the electronics" what do you mean? An asynchronization between the drive and the flux signals on the electronics themselves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An asynchronization between the drive and the flux signals on the electronics themselves?

Yes

src/qibocal/protocols/xyz_timing.py Outdated Show resolved Hide resolved
for duration in durations:
ro_pulses.append([])
for qubit in targets:
sequence = PulseSequence()
Copy link
Contributor

Choose a reason for hiding this comment

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

If you create the sequence at each iteration of the for loop over targets only the last qubit in target will be present in the sequence.

src/qibocal/protocols/xyz_timing.py Outdated Show resolved Hide resolved
src/qibocal/protocols/xyz_timing.py Outdated Show resolved Hide resolved
]
fit_parameters, perr = curve_fit(
fit_function,
delay - data.pulse_duration[qubit],
Copy link
Contributor

Choose a reason for hiding this comment

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

At this point I would just store directly this value in the data structure given that you also repeat the same line in the plotting function.


fig.update_layout(
showlegend=True,
xaxis_title="Time [ns]",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put a bit more details here.

src/qibocal/protocols/xyz_timing.py Outdated Show resolved Hide resolved
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.

2 participants