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

Raise an error for MCMs nested in Conditionals #7027

Merged
merged 11 commits into from
Mar 5, 2025
Merged

Conversation

lillian542
Copy link
Contributor

Context:
The current methods for of mid-circuit measurement processing don't support applying a mid-circuit measurement conditionally. This was not within the scope of the implementation for any of these methods, so we don't expect it to work - however, we would prefer to return clear error messages, and we don't want to be returning incorrect results.

Currently, qml.cond(m, qml.measure)(wire) either raises a difficult to parse error (in the case of deferred measurements and analytical execution), or returns incorrect results (in the case of tree-traversal, one-shot, and deferred measurements with shots).

Description of the Change:

We add validation in the midcircuit_measurements transform, checking for conditional mcms and raising an error. Specific mcm_methods can be made to pass this validation check in the future if support for conditional mcms are added to the method; for now, we disallow it completely.

Benefits:
We don't return incorrect results or raise cryptic errors.

Possible Drawbacks:
It adds one more iteration over the operations for validation 🤷‍♀️

Related GitHub Issues:
None, but sc stories [sc-84177] and [sc-84178]

This comment was marked as resolved.

@lillian542 lillian542 marked this pull request as ready for review February 28, 2025 20:29
Copy link

codecov bot commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (ab34d13) to head (0389335).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7027   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files         488      488           
  Lines       46807    46818   +11     
=======================================
+ Hits        46622    46634   +12     
+ Misses        185      184    -1     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mudit2812 mudit2812 self-requested a review March 3, 2025 21:49
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Thanks @lillian542
If the answer is no to my first question below, then no concerns with the rest, and will be fine to approve.

Copy link
Contributor

@andrijapau andrijapau left a comment

Choose a reason for hiding this comment

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

What if the measure is in a cond branch that has other operators around it, will this still detect it?

Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

Changing my review based on @mlxd 's observations

@lillian542
Copy link
Contributor Author

What if the measure is in a cond branch that has other operators around it, will this still detect it?

@andrijapau yes! On the tape we have a series of Conditional ops, each one with a single base.

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Thanks a bunch @lillian542
One suggestion, but happy to approve otherwise.

Co-authored-by: Lee James O'Riordan <[email protected]>
Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Thanks a lot @lillian542

@lillian542 lillian542 enabled auto-merge (squash) March 5, 2025 19:25
@lillian542 lillian542 merged commit b9c31a1 into master Mar 5, 2025
46 checks passed
@lillian542 lillian542 deleted the validate_mcm_in_cond branch March 5, 2025 19:43
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.

4 participants