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

Couplers Resonator Spec #565

Merged
merged 20 commits into from
Nov 2, 2023
Merged

Couplers Resonator Spec #565

merged 20 commits into from
Nov 2, 2023

Conversation

Jacfomg
Copy link
Contributor

@Jacfomg Jacfomg commented Oct 17, 2023

Draft routines, some loops missing and remove fitting from reused plots. Works with qibolab :qiboteam/qibolab#623 and qibolab_platforms []

Resonator:
image

Qubit:
imagen

Related paper: https://dspace.mit.edu/handle/1721.1/119507

https://arxiv.org/pdf/1912.10721.pdf

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

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #565 (ea04864) into main (158e7e6) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #565      +/-   ##
==========================================
+ Coverage   96.24%   96.27%   +0.02%     
==========================================
  Files          96       99       +3     
  Lines        6551     6705     +154     
==========================================
+ Hits         6305     6455     +150     
- Misses        246      250       +4     
Flag Coverage Δ
unittests 96.27% <100.00%> (+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%> (ø)
...cterization/couplers/coupler_qubit_spectroscopy.py 100.00% <100.00%> (ø)
...ization/couplers/coupler_resonator_spectroscopy.py 100.00% <100.00%> (ø)
...bocal/protocols/characterization/couplers/utils.py 100.00% <100.00%> (ø)
...rotocols/characterization/flux_dependence/utils.py 84.37% <100.00%> (+0.82%) ⬆️

... and 1 file with indirect coverage changes

@Jacfomg Jacfomg marked this pull request as ready for review October 18, 2023 08:08
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 @Jacfomg for implementing this.
You can find below some comments.
Mainly for improving the code.
I believe that there are also a lot of TODOs, are you planning to address those in this PR?

@Jacfomg
Copy link
Contributor Author

Jacfomg commented Oct 20, 2023

I will try to do as much work on new routines while this calibration lasts and I would go over them later, help is appreciated of course

@DavidSarlle
Copy link
Contributor

@Jacfomg @andrea-pasquale The missing point in this routines are the fittings. We should be able to extract information about the couplers points, at least what are the bias values where the interaction between qubits or qubit and coupler are present.

@Jacfomg
Copy link
Contributor Author

Jacfomg commented Oct 25, 2023

Sure, please work on that, I'm have other stuff I have to take off as lab stuff is not my work anymore.

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.

Another small comments

Comment on lines 27 to 28
measured_qubit: list[QubitId]
"""Qubit to readout from the pair"""
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the criteria for choosing which qubit to readout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't played a lot with those, ideally you will measure both but to avoid any crosstalk I got only one and the one that was not the center qubit to ensure I was getting the coupler I wanted and not something coming from another coupler.

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.

Another small comments

@Edoardo-Pedicillo
Copy link
Contributor

Edoardo-Pedicillo commented Nov 1, 2023

@Edoardo-Pedicillo @andrea-pasquale Should we delete this then?

No, the default register_qubit does not work with routines with 2D plots

Copy link
Contributor

@GabrielePalazzo GabrielePalazzo left a comment

Choose a reason for hiding this comment

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

From my side I think it's almost ready to be merged.

src/qibocal/protocols/characterization/couplers/utils.py Outdated Show resolved Hide resolved
@Jacfomg Jacfomg removed the request for review from DavidSarlle November 2, 2023 08:47
Copy link
Contributor

@GabrielePalazzo GabrielePalazzo left a comment

Choose a reason for hiding this comment

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

Seems ok to me.

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.

Another bunch of comments mainly improving documentation and plotting

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 @Jacfomg

@andrea-pasquale andrea-pasquale added this pull request to the merge queue Nov 2, 2023
Merged via the queue into main with commit 20fb7d0 Nov 2, 2023
21 checks passed
@andrea-pasquale andrea-pasquale deleted the coupler_spec branch November 2, 2023 16:53
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.

5 participants