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

Update CHSH pulses routine to 0.2 #1049

Open
wants to merge 5 commits into
base: 0.2
Choose a base branch
from
Open

Update CHSH pulses routine to 0.2 #1049

wants to merge 5 commits into from

Conversation

stavros11
Copy link
Member

Still need to confirm it works properly on hardware.

Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 93.43434% with 13 lines in your changes missing coverage. Please review.

Project coverage is 96.91%. Comparing base (c280398) to head (549b504).
Report is 4 commits behind head on 0.2.

Files with missing lines Patch % Lines
...cal/protocols/two_qubit_interaction/chsh/pulses.py 80.48% 8 Missing ⚠️
...l/protocols/two_qubit_interaction/chsh/protocol.py 97.85% 3 Missing ⚠️
...ocal/protocols/two_qubit_interaction/chsh/utils.py 86.66% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              0.2    #1049      +/-   ##
==========================================
- Coverage   97.02%   96.91%   -0.12%     
==========================================
  Files          98      102       +4     
  Lines        7893     8091     +198     
==========================================
+ Hits         7658     7841     +183     
- Misses        235      250      +15     
Flag Coverage Δ
unittests 96.91% <93.43%> (-0.12%) ⬇️

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

Files with missing lines Coverage Δ
src/qibocal/protocols/__init__.py 100.00% <ø> (ø)
...ibocal/protocols/two_qubit_interaction/__init__.py 100.00% <100.00%> (ø)
...l/protocols/two_qubit_interaction/chsh/__init__.py 100.00% <100.00%> (ø)
...ocal/protocols/two_qubit_interaction/chsh/utils.py 86.66% <86.66%> (ø)
...l/protocols/two_qubit_interaction/chsh/protocol.py 97.85% <97.85%> (ø)
...cal/protocols/two_qubit_interaction/chsh/pulses.py 80.48% <80.48%> (ø)

... and 1 file with indirect coverage changes

try:
return chsh / nshots
except ZeroDivisionError:
log.warning("Zero number of shots, returning zero.")
Copy link
Member

Choose a reason for hiding this comment

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

Drop the warning, just noise :P

Copy link
Member

Choose a reason for hiding this comment

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

If 0 is not a consistent limit value, consider returning nan. If it is consistent, no need to do anything :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the warning in 549b504, even though this code was just copy pasted from the main branch and not originally authored by me. Regarding 0, I am not sure if it is consistent, however it seems to me that the except branch is never going to be executed in practice, as I don't see why someone would launch this routine (or anything on hardware) with nshots == 0.

Copy link
Member

Choose a reason for hiding this comment

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

We could even consider nshots == 0, and just leave the original exception.

@stavros11 stavros11 mentioned this pull request Dec 5, 2024
76 tasks
@stavros11
Copy link
Member Author

Marking this as ready as I managed to get consistent results with 0.1 for B1-B3 pair:

0.1: http://login.qrccluster.com:9000/Pk4B8i3mRoaHOIWMDX7AWg==
0.2: http://login.qrccluster.com:9000/K4uyV9oBRxetDVfVp-a97w==

Note that I used qiboteam/qibolab#1116 for this.

@stavros11 stavros11 marked this pull request as ready for review December 7, 2024 18:55
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.

3 participants