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

Fix expansion of fragments tapes containing no MeasureNode #2223

Merged
merged 3 commits into from
Feb 22, 2022

Conversation

anthayes92
Copy link
Contributor

@anthayes92 anthayes92 commented Feb 22, 2022

Context:
Fragment tapes of cut circuit need to be expanded to multiple configurations in order to execute and recombine results. However, the observables of fragments tapes containing no MeasureNode were being lost.

Description of the Change:
This PR fixes the logic so that measurements of such fragments persist across the following configurations

Benefits:
Fixes broken pipeline for this specific case

Related GitHub Issues:
Blocker for #2216

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @anthayes92! Good spot! My only question is whether the bugfix should live in the _get_measurements helper function.

Comment on lines 527 to 532
if len(group) > 0:
measurements = _get_measurements(group, tape.measurements)
# This ensures the measurements of the original tape are carried over to the
# following tape configurations in the absence of any MeasureNodes in the fragment
else:
measurements = tape.measurements
Copy link
Contributor

Choose a reason for hiding this comment

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

We could put this logic in _get_measurements, i.e., the first thing we do is:

if len(group) == 0:
    return measurements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea! I've moved it in this commit 1129df6

@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #2223 (9f9418a) into master (8f59e4a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2223   +/-   ##
=======================================
  Coverage   99.26%   99.26%           
=======================================
  Files         231      231           
  Lines       18241    18243    +2     
=======================================
+ Hits        18107    18109    +2     
  Misses        134      134           
Impacted Files Coverage Δ
pennylane/transforms/qcut.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f59e4a...9f9418a. Read the comment docs.

@trbromley trbromley marked this pull request as ready for review February 22, 2022 23:02
@anthayes92 anthayes92 merged commit 206c8f5 into master Feb 22, 2022
@anthayes92 anthayes92 deleted the qcut-fix-expansion-obs-bug branch February 22, 2022 23:04
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.

None yet

2 participants