qiboteam / qibolab

Quantum hardware module and drivers for Qibo.
https://qibo.science
Apache License 2.0
40 stars 10 forks source link

Qblox recursion fix #831

Closed hay-k closed 3 months ago

hay-k commented 4 months ago

Fixes https://github.com/qiboteam/qibolab/issues/830

When the number of bins (nshots * total_number_of_sweep_points) is larger than the maximum supported by the instrument (2**17), the execution is done in batches. Previously the _sweep_recursion was called which resulted into infinite recursion stack.

Checklist:

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 92.59259% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 66.14%. Comparing base (d080141) to head (b758fd4). Report is 3 commits behind head on main.

Files Patch % Lines
src/qibolab/instruments/qblox/controller.py 92.59% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #831 +/- ## ========================================== + Coverage 65.09% 66.14% +1.04% ========================================== Files 50 50 Lines 6034 6035 +1 ========================================== + Hits 3928 3992 +64 + Misses 2106 2043 -63 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibolab/pull/831/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/qiboteam/qibolab/pull/831/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `66.14% <92.59%> (+1.04%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

biankawolo commented 4 months ago

hey! I m checking it right now!


From: Hayk Sargsyan @.> Sent: Monday, March 11, 2024 10:11 AM To: qiboteam/qibolab @.> Cc: Bianka Woloncewicz @.>; Review requested @.> Subject: Re: [qiboteam/qibolab] Qblox recursion fix (PR #831)

You don't often get email from @.*** Learn why this is importanthttps://protect.checkpoint.com/v2/___https://aka.ms/LearnAboutSenderIdentification___.bWVjMTp0ZWNobm9sb2d5aW5ub3ZhdGlvbmluc3RpdHV0ZTpjOm86NmNiY2E5YjA2MmRhYmZiNDdhYTg2MWYzNDlmZjFjMjg6NjoxYWI5OjY0NTQ3ZGE4YzI5OTM3ZThhMzZlNmY0MDc4M2FkZDQ5ZmUxYTI2NjE2NTk4NTBlYTk5MzJkODA2MmVhYWI1M2I6aDpU

@hay-k commented on this pull request.


In src/qibolab/instruments/qblox/controller.pyhttps://protect.checkpoint.com/v2/___https://github.com/qiboteam/qibolab/pull/831%23discussion_r1519196548___.bWVjMTp0ZWNobm9sb2d5aW5ub3ZhdGlvbmluc3RpdHV0ZTpjOm86NmNiY2E5YjA2MmRhYmZiNDdhYTg2MWYzNDlmZjFjMjg6NjozN2YzOjg5MTYxODMxMDZmNjQ2MGY2ZWY1NTllNjYwNWE3MTBiNWM2NmJiMmViNjBlNWE5NjZiNzgwNGI1ZGM1YjM2ZjU6aDpU:

  • self._sweep_recursion(
  • qubits,
  • sequence,
  • options,
  • *((split_sweeper,) + sweepers[1:]),
  • results=results,
  • )
  • @staticmethod
  • def _combine_result_chunks(result_chunks):

I am thinking of implementing this functionality as some sort of append method for data types IntegratedResults and SampleResults, instead of doing the gymnastics here. The existing methods of those classes make them look like they were designed to be immutable. Is this impression true? If no, I guess just a simple append method that changes each object in place is enough. If yes, I can implement classmethods that accept multiple instances and create a new instance containing the appended (across a certain axis) data. Any comments?

— Reply to this email directly, view it on GitHubhttps://protect.checkpoint.com/v2/___https://github.com/qiboteam/qibolab/pull/831%23pullrequestreview-1926991754___.bWVjMTp0ZWNobm9sb2d5aW5ub3ZhdGlvbmluc3RpdHV0ZTpjOm86NmNiY2E5YjA2MmRhYmZiNDdhYTg2MWYzNDlmZjFjMjg6Njo5MmIzOmIyYTdhMzBlMjgwMDkwMzY5MjY3ZDNiMzYwYzE4YTY0ZTg4MTcwMThlYjcyMmJlZDdjODI0NDZkMDMzNTRlYjY6aDpU, or unsubscribehttps://protect.checkpoint.com/v2/___https://github.com/notifications/unsubscribe-auth/BE3BN26UVMCIXMAYO6NE3WDYXVDHZAVCNFSM6AAAAABEM42IWWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMRWHE4TCNZVGQ___.bWVjMTp0ZWNobm9sb2d5aW5ub3ZhdGlvbmluc3RpdHV0ZTpjOm86NmNiY2E5YjA2MmRhYmZiNDdhYTg2MWYzNDlmZjFjMjg6NjowOGZhOjRhZDU3NmFhZjU0ZDE0YjBlZmU0NmZkYWM5ZDYwMWZkNTlmZWQ5OTA2MjUyNmNhNWYxZDRmNGZhYWZhMGNkZDY6aDpU. You are receiving this because your review was requested.Message ID: @.***>