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

Adding hash to Custom #1101

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Adding hash to Custom #1101

merged 1 commit into from
Nov 15, 2024

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Nov 14, 2024

Closes #1100.

Checklist:

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

Copy link

codecov bot commented Nov 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.87%. Comparing base (ed8b3cc) to head (5550a10).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1101      +/-   ##
==========================================
+ Coverage   50.84%   50.87%   +0.03%     
==========================================
  Files          63       63              
  Lines        2903     2905       +2     
==========================================
+ Hits         1476     1478       +2     
  Misses       1427     1427              
Flag Coverage Δ
unittests 50.87% <100.00%> (+0.03%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. I have not tested on hardware but I assume it fixes the issue with QM. I also confirm that the reason hash is used in the driver instead of id is exactly what is written in #1100 (comment).

Not related at all to this PR, but id should still be used in at least two places: 1. when registering acquisition variables, because every measurement has its own acquisition, even when two measurements have the same waveform for probe, 2. when registering sweepers, because each sweeper affects specific pulses, even if there are other pulses in the sequence with the same waveform. In other cases, the driver is using hash internally:

def operation(pulse):
"""Generate operation name in QM ``config`` for the given pulse."""
return str(hash(pulse))

to map pulses to operations in the QM config, to avoid duplication in uploaded waveforms.

@andrea-pasquale andrea-pasquale merged commit 5836076 into main Nov 15, 2024
40 checks passed
@andrea-pasquale andrea-pasquale deleted the envelope_hash branch December 15, 2024 09:39
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.

Unable to play custom pulses with QM
3 participants