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

Add MidMeasureMP in LightningQubit (cpp only) #645

Merged
merged 8 commits into from
Mar 18, 2024

Conversation

tomlqc
Copy link
Contributor

@tomlqc tomlqc commented Mar 15, 2024

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    tests directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the .github/CHANGELOG.md file, summarizing the
    change, and including a link back to the PR.

  • Ensure that code is properly formatted by running make format.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context:
Native-execution of circuits with mid-measurements is enabled via the dynamic-one-shot transform. To support the transform, a device must implement methods to apply MidMeasureMP.

MidMeasureMP is a special kind of operation where a sample is drawn from the state vector probability distribution and a projector is applied to the state vector accordingly. It may also support options like reset and post-selection.

The most straightforward way to deal with the requirements is to enhance the state vector class StateVectorLQubit.

This PR is only about the C++ part.

Description of the Change:

Benefits:

Possible Drawbacks:

Related GitHub Issues:

Related GitHub Pull Request:
#621 - See discussions

Related Shortcut Story:
[sc-57704]

@tomlqc tomlqc requested a review from AmintorDusko March 15, 2024 19:23
@tomlqc tomlqc requested a review from vincentmr March 15, 2024 19:24
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Can we have some tests for unit tests for normalize?

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.71%. Comparing base (a3071d3) to head (4022910).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #645      +/-   ##
==========================================
+ Coverage   92.98%   98.71%   +5.72%     
==========================================
  Files          14      173     +159     
  Lines        2368    24506   +22138     
==========================================
+ Hits         2202    24192   +21990     
- Misses        166      314     +148     

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

@tomlqc
Copy link
Contributor Author

tomlqc commented Mar 15, 2024

Can we have some tests for unit tests for normalize?

Currently it's tested as part of collapse(). But, yes, of course we can have separated tests. I just would have preferred getting the feedback on the previous review.

Actually I may add the separate tests when I commit the Kokkos version, would this be ok?

@AmintorDusko
Copy link
Contributor

Can we have some tests for unit tests for normalize?

Currently it's tested as part of collapse(). But, yes, of course we can have separated tests. I just would have preferred getting the feedback on the previous review.

Actually I may add the separate tests when I commit the Kokkos version, would this be ok?

@tomlqc, we didn't see it before. It is healthy for our software stack to have unit tests, that's how we get the small problems before they turn into big problems.
I will approve for now, and you can proceed with the tests in the next PR.

@tomlqc
Copy link
Contributor Author

tomlqc commented Mar 15, 2024

Thanks @AmintorDusko I understand your point. I'll make sure to add the unittest in the next PR.

.github/CHANGELOG.md Outdated Show resolved Hide resolved
vincentmr and others added 2 commits March 18, 2024 08:17
Co-authored-by: Vincent Michaud-Rioux <vincentm@nanoacademic.com>
Copy link
Contributor

@vincentmr vincentmr left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @tomlqc .

@tomlqc tomlqc merged commit 5b4ef77 into master Mar 18, 2024
85 checks passed
@tomlqc tomlqc deleted the feature/mid-measure-qubit branch March 18, 2024 20:45
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