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

Adding ramsey sequences signal protocol #674

Closed
wants to merge 2 commits into from

Conversation

andrea-pasquale
Copy link
Contributor

Closes #673.
This protocol could be useful to run long scans without using sweepers. It might be replaced by the unrolled version #666 soon.

@GabrielePalazzo @PiergiorgioButtarini whenever you have time could test it on qblox to see if the output is the same?
Here is an example of a runcard:

actions:
    - id: ramsey
      priority: 0
      operation: ramsey_sequences_signal #ramsey_signal
      # main: single shot classification
      parameters:
        delay_between_pulses_start: 16 # must be a multiple of 4 incl 0
        delay_between_pulses_end: 200
        delay_between_pulses_step: 25 # must be a multiple of 4
        n_osc: 5

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 18, 2023

Codecov Report

Merging #674 (d82adee) into main (f9dc958) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #674      +/-   ##
==========================================
- Coverage   96.05%   96.03%   -0.03%     
==========================================
  Files         106      108       +2     
  Lines        7355     7386      +31     
==========================================
+ Hits         7065     7093      +28     
- Misses        290      293       +3     
Flag Coverage Δ
unittests 96.03% <100.00%> (-0.03%) ⬇️

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%> (ø)
...ocal/protocols/characterization/ramsey/__init__.py 100.00% <100.00%> (ø)
...ibocal/protocols/characterization/ramsey/ramsey.py 100.00% <100.00%> (ø)
...tocols/characterization/ramsey/ramsey_sequences.py 100.00% <ø> (ø)
...characterization/ramsey/ramsey_sequences_signal.py 100.00% <100.00%> (ø)
...protocols/characterization/ramsey/ramsey_signal.py 97.16% <100.00%> (ø)

... and 1 file with indirect coverage changes

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.

Everything seems ok to me (tested on qblox).

@stavros11
Copy link
Member

Out of curiosity, is this only needed for doing non-detuned Ramsey with sequences without sweepers? Because last week Boulos also contacted me about a similar issue and I remember that using the existing Ramsey would fall back to sequences if I set n_osc > 0.

@andrea-pasquale
Copy link
Contributor Author

Out of curiosity, is this only needed for doing non-detuned Ramsey with sequences without sweepers? Because last week Boulos also contacted me about a similar issue and I remember that using the existing Ramsey would fall back to sequences if I set n_osc > 0.

When he spoke to me I think that the major issue was the fact that we wanted to run long scans, therefore he needed to use the *_sequences protocol, showing on the y axis the voltages. Before this PR we were supporting only one ramsey protocol with sequences that was using probabilities and not signal. I don't know if he needs to run a detuned one or not.

@stavros11
Copy link
Member

stavros11 commented Dec 20, 2023

Yes, I think the existing ramsey_signal uses sweeper when n_osc == 0 and sequences when n_osc > 0:

So in the end it should be equivalent to the new routine added here for n_osc > 0.

@andrea-pasquale
Copy link
Contributor Author

Oh yes, you are right. I will double check with @Boulos-Alfakes and I can close the PR.

Copy link
Contributor

@Edoardo-Pedicillo Edoardo-Pedicillo left a comment

Choose a reason for hiding this comment

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

Thank @andrea-pasquale, LGTM

@andrea-pasquale
Copy link
Contributor Author

I think that probably we will not need this PR. Let me close it for now and if it will be necessary I will open the PR again.

@andrea-pasquale andrea-pasquale deleted the ramsey_sequences_signal branch June 10, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restore ramsey_sequence with signal
5 participants