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

Sequence unrolling in allXY #629

Merged
merged 6 commits into from
Dec 11, 2023
Merged

Sequence unrolling in allXY #629

merged 6 commits into from
Dec 11, 2023

Conversation

stavros11
Copy link
Member

@stavros11 stavros11 commented Nov 14, 2023

Similar to #610 but for allXY. Also requires qiboteam/qibolab#618 but I did not change the pyproject since we will wait for the qibolab release anyway.

@GabrielePalazzo @Jacfomg if you have calibrated/used iqm5q recently could you please check if this works for you? In principle you don't need calibrated qubits, we just need to confirm that the behavior is the same with unrolling: True and unrolling: False. On qw5q_gold I get the same behavior and execution time drops from 65 to 11 seconds.

Note that here I changed np.array([z_proj]) back to np.array(z_proj) so that it works for the dummy and Zurich that return the probability as array. For qblox you need to change back to np.array([z_proj]). This is regardless of whether you use unrolling or no (see qiboteam/qibolab#647).

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: unrolling
    • Qibolab_platforms_qrc: main

@stavros11 stavros11 added the qibolab release required PR requires qibolab feature not available in the current poetry.lock label Nov 14, 2023
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Merging #629 (42987a4) into main (1fe5cf0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #629   +/-   ##
=======================================
  Coverage   96.05%   96.05%           
=======================================
  Files         106      106           
  Lines        7342     7352   +10     
=======================================
+ Hits         7052     7062   +10     
  Misses        290      290           
Flag Coverage Δ
unittests 96.05% <100.00%> (+<0.01%) ⬆️

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

Files Coverage Δ
.../qibocal/protocols/characterization/allxy/allxy.py 100.00% <100.00%> (ø)

Copy link
Contributor

@Jacfomg Jacfomg left a comment

Choose a reason for hiding this comment

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

We can't test the same behavior with zurich until we fix the sequences on zurich. But, since the unrolling is now in qibolab we can merge, right ?

@stavros11 stavros11 removed qibolab release required PR requires qibolab feature not available in the current poetry.lock deploy on hardware labels Dec 7, 2023
@stavros11
Copy link
Member Author

We can't test the same behavior with zurich until we fix the sequences on zurich. But, since the unrolling is now in qibolab we can merge, right ?

Yes, I updated poetry to use the latest qibolab release. Also, this seems to work with qblox, so I would say it is ready. It is an enhancement, so if you use unrolling: False (which is the default), it should fall back to the original allXY without unrolling.

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.

Thanks @stavros11.
I confirm that it works as expected both on qblox and zh.

As a small note it would be nice to propagate this option also for the allxy_drag_pulse_tuning protocol. We can also do it in a separate PR and not here if you prefer not do it now.

tests/runcards/protocols.yml Outdated Show resolved Hide resolved
@stavros11
Copy link
Member Author

Thank you for the reviews.

As a small note it would be nice to propagate this option also for the allxy_drag_pulse_tuning protocol. We can also do it in a separate PR and not here if you prefer not do it now.

Unless this routine is used a lot and we would really benefit by doing unrolling (which I don't think so), I would prefer to postpone because after some discussions with @alecandido we may change how the results are returned by platform.execute_pulse_sequences and this may break this implementation. This probably won't happen before qibolab 0.1.6 (or later), but still better having to update one place than many.

@stavros11 stavros11 added this pull request to the merge queue Dec 11, 2023
Merged via the queue into main with commit 5f287ce Dec 11, 2023
21 checks passed
@stavros11 stavros11 deleted the allXYunrolling branch December 11, 2023 12:00
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