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: Support single-register measurements in from_ir #934

Merged
merged 3 commits into from
Apr 5, 2024
Merged

Conversation

ashlhans
Copy link
Contributor

@ashlhans ashlhans commented Apr 2, 2024

Issue #, if available:
Currently, the statement b = measure q in an OpenQASM program results in the following error:

ValueError: Operator qubit count 1 must be equal to size of target qubit set QubitSet([Qubit(0), Qubit(1)])

This is because a measure instruction is always a 1 qubit instruction but right now, all the targets get added to the measure instruction even if there are more than 1 qubit measured.

To reproduce this issue:

bell_qasm = """
OPENQASM 3;

qubit[2] q;
bit[2] c;

h q[0];
cnot q[0], q[1];

c = measure q;
"""

from braket.circuits import Circuit
print(Circuit.from_ir(bell_qasm))

Description of changes:

  • Add a loop to add a single measure instruction for each qubit target.
  • Add a test to prevent this error in the future

Testing done:
tox and tox -e integ-tests

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ashlhans ashlhans self-assigned this Apr 2, 2024
@ashlhans ashlhans requested a review from a team as a code owner April 2, 2024 00:32
@ashlhans ashlhans added the bug Something isn't working label Apr 2, 2024
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (c57a9ba) to head (48bd56a).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #934   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          133       133           
  Lines         8905      8906    +1     
  Branches      2006      2007    +1     
=========================================
+ Hits          8905      8906    +1     

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

@ashlhans ashlhans requested review from krneta and kshitijc April 2, 2024 18:01
@@ -799,6 +799,24 @@ def test_from_ir_with_measure():
assert Circuit.from_ir(source=ir.source, inputs=ir.inputs) == expected_circ


def test_from_ir_with_single_measure():
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add tests to do a round trip transformation? Circuit -> OQ3 -> Circuit.from_ir and assert that the original and the parsed circuit are the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a round trip test.

This test_from_ir_with_single_measure test would not have the same OpenQASM output though because Circuit.from_ir would not output b = measure q since the BDK does not handle the measure assignment.

So instead it would output:

b[0] = measure q[0];
b[1] = measure q[1];

@speller26 speller26 changed the title fix: add support for measuring a single register in an OpenQASM program fix: Support single-register measurements in from_ir Apr 2, 2024
@ashlhans ashlhans requested a review from mbeach-aws April 4, 2024 16:21
Comment on lines +851 to +852
assert new_ir == ir
assert Circuit.from_ir(source=ir.source, inputs=ir.inputs) == circuit
Copy link
Member

Choose a reason for hiding this comment

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

This is already covered by other test cases, right? Not that I'm complaining.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is covered by other test cases

@ashlhans ashlhans merged commit b98bce5 into main Apr 5, 2024
24 checks passed
@ashlhans ashlhans deleted the fixFromIrBug branch April 5, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants