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

Bell protocol #452

Merged
merged 34 commits into from
Sep 28, 2023
Merged

Bell protocol #452

merged 34 commits into from
Sep 28, 2023

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Jul 31, 2023

This PR implements the following:

  • CHSH as protocol (with pulses or circuits)
  • ReadoutMitigationMatrix protocol which can be used by itself or directly in CHSH by setting apply_readout_mitigation=True

TODO:

  • Implement readout error mitigation
  • Run on hardware

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 Jul 31, 2023

Codecov Report

Merging #452 (cbd8707) into main (aef9c74) will increase coverage by 0.19%.
The diff coverage is 97.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #452      +/-   ##
==========================================
+ Coverage   96.51%   96.71%   +0.19%     
==========================================
  Files          67       74       +7     
  Lines        4998     5380     +382     
==========================================
+ Hits         4824     5203     +379     
- Misses        174      177       +3     
Flag Coverage Δ
unittests 96.71% <97.96%> (+0.19%) ⬆️

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

Files Changed Coverage
.../characterization/two_qubit_interaction/chevron.py 33.33%
...racterization/two_qubit_interaction/cz_virtualz.py 90.00%
...cols/characterization/readout_mitigation_matrix.py 96.15%
...cterization/two_qubit_interaction/chsh/protocol.py 99.28%
src/qibocal/auto/operation.py 100.00%
src/qibocal/auto/runcard.py 100.00%
src/qibocal/auto/task.py 100.00%
src/qibocal/protocols/characterization/__init__.py 100.00%
...characterization/two_qubit_interaction/__init__.py 100.00%
...cterization/two_qubit_interaction/chsh/__init__.py 100.00%
... and 5 more

@andrea-pasquale andrea-pasquale marked this pull request as ready for review August 3, 2023 16:14
@andrea-pasquale
Copy link
Contributor Author

This PR should be ready to review now.
We are still not able see Bell with qw5q_gold, I have gone over the code with @igres26 and it seems to be working as expected on paper.
I won't be able to follow up this PR during the next two weeks since I will be on vacations. As soon as I'm back I can implement the suggested changes.

@andrea-pasquale
Copy link
Contributor Author

andrea-pasquale commented Aug 22, 2023

@andrea-pasquale I think you need to merge main into this PR in order to have the latest changes. An error is raised because of the latest renaming of the dump function to dump_runcard in the platform.

Traceback (most recent call last): File "/home/users/david.fuentes/.conda/envs/auto/bin/qq", line 6, in sys.exit(command()) File "/home/users/david.fuentes/.conda/envs/auto/lib/python3.9/site-packages/click/core.py", line 1130, in call return self.main(*args, **kwargs) File "/home/users/david.fuentes/.conda/envs/auto/lib/python3.9/site-packages/click/core.py", line 1055, in main rv = self.invoke(ctx) File "/home/users/david.fuentes/.conda/envs/auto/lib/python3.9/site-packages/click/core.py", line 1404, in invoke return ctx.invoke(self.callback, **ctx.params) File "/home/users/david.fuentes/.conda/envs/auto/lib/python3.9/site-packages/click/core.py", line 760, in invoke return __callback(*args, **kwargs) File "/nfs/users/david.fuentes/qibocal/src/qibocal/cli/_base.py", line 58, in command builder = ActionBuilder(card, folder, force, update=update) File "/nfs/users/david.fuentes/qibocal/src/qibocal/cli/builders.py", line 38, in init self.meta = self._prepare_output(runcard) File "/nfs/users/david.fuentes/qibocal/src/qibocal/cli/builders.py", line 76, in _prepare_output self.platform.dump(self.folder / PLATFORM) AttributeError: 'Platform' object has no attribute 'dump'

You are right, I've just merged main now.

@DavidSarlle
Copy link
Contributor

@andrea-pasquale thanks for implementing the routine. I have reviewed the code and there are no comments regarding the code, it works as expected and it is very straightforward. Also thanks for implementing the possibility to run the routine using pulses or circuits. But regarding that, the results obtained using the same conditions with pulses and circuits are very different as you can see below:

imagen

imagen

Both executions has been run with the same conditions (qubits, runcard and action parameters). To me seems that there is a source of errors in the circuit execution. Reviewing the code I am pretty sure that there is no problem in the routine and the errors could come from:

  1. Transpilation of the circuit into gates and pulses. Maybe we should check it @Simone-Bordoni
  2. Pi/2 pulse characterization. Right now there are no routines specific for characterization of the pi/2 pulses and the transpiler transform circuits into U3 gates that use pi/2 pulses. That could be also a source of errors @igres26 @Simone-Bordoni

Can you run the same test and see if you obtain the similar results @andrea-pasquale @igres26 @Simone-Bordoni, in order to determine if we have an error in any of the steps mentioned?

@andrea-pasquale
Copy link
Contributor Author

@andrea-pasquale thanks for implementing the routine. I have reviewed the code and there are no comments regarding the code, it works as expected and it is very straightforward. Also thanks for implementing the possibility to run the routine using pulses or circuits. But regarding that, the results obtained using the same conditions with pulses and circuits are very different as you can see below:

imagen

imagen

Both executions has been run with the same conditions (qubits, runcard and action parameters). To me seems that there is a source of errors in the circuit execution. Reviewing the code I am pretty sure that there is no problem in the routine and the errors could come from:

  1. Transpilation of the circuit into gates and pulses. Maybe we should check it @Simone-Bordoni
  2. Pi/2 pulse characterization. Right now there are no routines specific for characterization of the pi/2 pulses and the transpiler transform circuits into U3 gates that use pi/2 pulses. That could be also a source of errors @igres26 @Simone-Bordoni

Can you run the same test and see if you obtain the similar results @andrea-pasquale @igres26 @Simone-Bordoni, in order to determine if we have an error in any of the steps mentioned?

@DavidSarlle I found a bug in the code that was executing the readout mitigation matrix using circuits.
Here is the current output:

image

I believe that now the behavior is more or less consistent. There are some differences but I think that they are expected.

@DavidSarlle
Copy link
Contributor

@andrea-pasquale I can confirm that now the code works as expected.
As you mentioned, the results are slightly different but it is expected.

@andrea-pasquale andrea-pasquale linked an issue Aug 25, 2023 that may be closed by this pull request
@Edoardo-Pedicillo Edoardo-Pedicillo mentioned this pull request Aug 29, 2023
18 tasks
@andrea-pasquale andrea-pasquale added this to the Qibocal 0.0.4 milestone Aug 29, 2023
@andrea-pasquale
Copy link
Contributor Author

@stavros11 if possible I would like to merge also this PR given that it has been ready for a while and now the fridge is warm so we won't be able to test it.
Could you please have a look whenever you have time?

@andrea-pasquale andrea-pasquale added this pull request to the merge queue Sep 28, 2023
Merged via the queue into main with commit aa89d58 Sep 28, 2023
20 checks passed
@andrea-pasquale andrea-pasquale deleted the bell_protocol branch September 28, 2023 14:05
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.

Readout Matrix Routine
6 participants