-
Notifications
You must be signed in to change notification settings - Fork 6
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
Interleaved RB #864
Interleaved RB #864
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #864 +/- ##
==========================================
+ Coverage 97.48% 97.50% +0.02%
==========================================
Files 122 123 +1
Lines 9650 9701 +51
==========================================
+ Hits 9407 9459 +52
+ Misses 243 242 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
for more information, see https://pre-commit.ci
src/qibocal/protocols/randomized_benchmarking/standard_rb_2q_inter.py
Outdated
Show resolved
Hide resolved
qubits = data.pairs | ||
results = fit(qubits, data) | ||
|
||
# FIXME: I can only get the data.fidelity if there is an acquisition step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In which cases you don't have an acquisition step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when you just want to plot becuase you ran a new 2qRB but not interleaved for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the problem here. Either you run a 2qRb or a 2qInterleavedR, no?
new_circuit = Circuit(nqubits) | ||
if nqubits == 1: | ||
new_circuit.add(new_layer) | ||
elif nqubits == 2: | ||
for gate in new_layer: | ||
new_circuit.add(gate) | ||
|
||
random_indexes.append(random_index) | ||
# FIXME: General interleave |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raise a not supported error instead of a FIXME
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
interleaved_clifford = rb_gen.two_qubit_cliffords["13"] | ||
interleaved_clifford_gate = clifford2gates(interleaved_clifford) | ||
new_circuit.add(interleaved_clifford_gate) | ||
random_indexes.append("13") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just create the CZ with Qibo ? Since the 2qRB files are merely one possibility, let's make every effort to separate the Qibocal library from them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, here it should be better to create the CZ with Qibo
for more information, see https://pre-commit.ci
… into 2q_rb_interleaved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank @Jacfomg, just small comments
src/qibocal/protocols/randomized_benchmarking/standard_rb_2q_inter.py
Outdated
Show resolved
Hide resolved
src/qibocal/protocols/randomized_benchmarking/standard_rb_2q_inter.py
Outdated
Show resolved
Hide resolved
…nter.py Co-authored-by: Edoardo Pedicillo <[email protected]>
…nter.py Co-authored-by: Edoardo Pedicillo <[email protected]>
… into 2q_rb_interleaved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please fix the conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Jacfomg, mostly it looks good to me.
There are some conversations that were resolved but the comments from the reviewer were not addressed.
Once we address the comment we can merge.
It would also nice to run a very short RB on hardware. You can use for example: qiboteam/qibolab_platforms_qrc#160
qubits = data.pairs | ||
results = fit(qubits, data) | ||
|
||
# FIXME: I can only get the data.fidelity if there is an acquisition step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the problem here. Either you run a 2qRb or a 2qInterleavedR, no?
new_circuit = Circuit(nqubits) | ||
if nqubits == 1: | ||
new_circuit.add(new_layer) | ||
elif nqubits == 2: | ||
for gate in new_layer: | ||
new_circuit.add(gate) | ||
|
||
random_indexes.append(random_index) | ||
# FIXME: General interleave |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
interleaved_clifford = rb_gen.two_qubit_cliffords["13"] | ||
interleaved_clifford_gate = clifford2gates(interleaved_clifford) | ||
new_circuit.add(interleaved_clifford_gate) | ||
random_indexes.append("13") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, here it should be better to create the CZ with Qibo
Indeed, here it should be better to create the CZ with Qibo Why, it would be the same. And we keep everything safe in case Qibo does something different for another gate., |
Also, for the general interleave what would be the interface for which clifford to interleave, a number from 0 to 11000 as a parameter ? |
If the number is documented properly yes. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates @Jacfomg.
I would like to see a short run on hardware to check that everything is working properly before merging.
1st draft for the interleaved RB. It seems to work but it needs qiboteam/qibolab#906
Checklist:
master
main
main