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

rb_correction_qua #960

Closed
wants to merge 25 commits into from
Closed

rb_correction_qua #960

wants to merge 25 commits into from

Conversation

Jacfomg
Copy link
Contributor

@Jacfomg Jacfomg commented Aug 1, 2024

I dont know how to point this to two branches or if it's even possible so I merge rb_qua branch here

It needs #959 to be merged

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
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.

I've added some comments just for the script

Comment on lines 28 to 60
biases = np.arange(-0.2, 0.1, 0.01)
"bias points to sweep"

# Flipping
nflips_max = 200
"""Maximum number of flips ([RX(pi) - RX(pi)] sequences). """
nflips_step = 10
"""Flip step."""

# Ramsey signal
detuning = 3_000_000
"""Frequency detuning [Hz]."""
delay_between_pulses_start = 16
"""Initial delay between RX(pi/2) pulses in ns."""
delay_between_pulses_end = 5_000
"""Final delay between RX(pi/2) pulses in ns."""
delay_between_pulses_step = 200
"""Step delay between RX(pi/2) pulses in ns."""

# Std Rb Ondevice Parameters
num_of_sequences: int = 100
max_circuit_depth: int = 250
"Maximum circuit depth"
delta_clifford: int = 50
"Play each sequence with a depth step equals to delta_clifford"
seed: Optional[int] = 1234
"Pseudo-random number generator seed"
n_avg: int = 128
"Number of averaging loops for each random sequence"
save_sequences: bool = True
apply_inverse: bool = True
state_discrimination: bool = True
"Flag to enable state discrimination if the readout has been calibrated (rotated blobs and threshold)"
Copy link
Contributor

Choose a reason for hiding this comment

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

If those parameters are only used once perhaps I would just used directly during the protocol calls instead of adding 30 lines of code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was cleaner to change them there than to go aorund looking for them

runcards/rb_correction_qua.py Outdated Show resolved Hide resolved
runcards/rb_correction_qua.py Outdated Show resolved Hide resolved
@andrea-pasquale
Copy link
Contributor

I dont know how to point this to two branches or if it's even possible so I merge rb_qua branch here

Perhaps the easiest solution is to point to #959 and merge #949 afterwards.
I'm saying this also because if #959 doesn't get merged also this one cannot be merged.

@alecandido
Copy link
Member

I dont know how to point this to two branches or if it's even possible so I merge rb_qua branch here

@Jacfomg I can tell you, it is not xD

@Jacfomg
Copy link
Contributor Author

Jacfomg commented Aug 1, 2024

I dont know how to point this to two branches or if it's even possible so I merge rb_qua branch here

@Jacfomg I can tell you, it is not xD

Someone should make an issue for this XD

@Jacfomg
Copy link
Contributor Author

Jacfomg commented Aug 1, 2024

I dont know how to point this to two branches or if it's even possible so I merge rb_qua branch here

Perhaps the easiest solution is to point to #959 and merge #949 afterwards.

I'm saying this also because if #959 doesn't get merged also this one cannot be merged.

I did it the other way around, it should be fine too I hope

@alecandido
Copy link
Member

I did it the other way around should be fine too

As a temporary solution, I'd say it's definitely fine. I will try to merge #949 asap, at which point I will rebase #959, and you could rebase your branch there.

Someone should make an issue for this XD

Unfortunately, they do not accept issues :P
https://github.com/git/git
But you could write them an email :)

@Jacfomg Jacfomg added the do not merge Soft warning to avoid merging a PR for reason provided in the PR label Aug 2, 2024
@Jacfomg Jacfomg requested a review from igres26 August 2, 2024 06:08
@Jacfomg
Copy link
Contributor Author

Jacfomg commented Aug 2, 2024

@wilkensJ @ingoroth This would be the code to run some kind of RB corrections when simulating flux changes in the qubit. Feel free to play with it to generate the data you need. Also, you can add other routines to have a longer correction or a smaller one.

@andrea-pasquale andrea-pasquale added the experiment Related to an individual protocol label Aug 12, 2024
Base automatically changed from exec-ctx-manager to main August 13, 2024 09:43
An error occurred while trying to automatically change base from exec-ctx-manager to main August 13, 2024 09:43
@andrea-pasquale
Copy link
Contributor

Closing in favor of #975

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Soft warning to avoid merging a PR for reason provided in the PR experiment Related to an individual protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants