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

Fast reset Zurich #506

Merged
merged 11 commits into from
Jul 24, 2023
Merged

Fast reset Zurich #506

merged 11 commits into from
Jul 24, 2023

Conversation

Jacfomg
Copy link
Contributor

@Jacfomg Jacfomg commented Jun 26, 2023

I broke the way this was working before.

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

@Jacfomg Jacfomg requested a review from stavros11 June 26, 2023 15:05
@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.11 🎉

Comparison is base (e26449f) 60.18% compared to head (93fbcb5) 60.30%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
+ Coverage   60.18%   60.30%   +0.11%     
==========================================
  Files          40       40              
  Lines        6339     6348       +9     
==========================================
+ Hits         3815     3828      +13     
+ Misses       2524     2520       -4     
Flag Coverage Δ
unittests 60.30% <100.00%> (+0.11%) ⬆️

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

Impacted Files Coverage Δ
src/qibolab/platform.py 79.79% <ø> (ø)
src/qibolab/instruments/zhinst.py 75.72% <100.00%> (+2.26%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Jacfomg
Copy link
Contributor Author

Jacfomg commented Jun 28, 2023

@stavros11 A couple things, NativePulse has the attribute pulse_type while Pulse has the attribute type. And also Pulse.shape returns a PulseShape that has attributes like rel_sigma for a gaussian pulse while NativePulse.shape returns a string. Could we choose one of the two for both cases ?

@Jacfomg Jacfomg marked this pull request as draft June 28, 2023 09:39
@stavros11
Copy link
Member

@stavros11 A couple things, NativePulse has the attribute pulse_type while Pulse has the attribute type. And also Pulse.shape returns a PulseShape that has attributes like rel_sigma for a gaussian pulse while NativePulse.shape returns a string. Could we choose one of the two for both cases ?

Sorry, I just saw this. NativePulse is just (almost) a container for what is loaded from the native gates section in the runcard. It was not intended to use it for execution directly, it has to first be converted to a Pulse. This can be done with the NativePulse.pulse() method, but requires to specify the start and (optionally) relative_phase. If you think there are modifications that can make NativePulse more convenient we can also change it though.

@Jacfomg Jacfomg marked this pull request as ready for review July 12, 2023 08:09
@Jacfomg Jacfomg merged commit 53b860c into main Jul 24, 2023
@Jacfomg Jacfomg deleted the zurich-fix-fast-reset branch July 24, 2023 13:19
@alecandido alecandido mentioned this pull request Sep 22, 2023
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