feature: `add_verbatim_marker()` method to `BraketProgramContext` class for updated `Circuit.from_ir`
This PR and PR 285 were created to resolve Issue #972
Description of changes:
In this PR, the following change was made to support the updated Circuit.from_ir method:
- The class
BraketProgramContext(AbstractProgramContext)was updated to include the methodadd_verbatim_marker(), it invokes:Instruction(StartVerbatimBox(), target=[])Instruction(EndVerbatimBox(), target=[])
These instructions are called when instances of VerbatimBoxStart and VerbatimBoxEnd are encountered.
add_verbatim_marker() method overrides the similarly named method from AbstractProgramContext, introduced in the amazon-braket-default-simulator-python repository in PR 285
This PR also includes a unit test for the changes to from_ir
Testing done:
A test function test_from_ir_with_verbatim_box() was added to test_circuit.py to verify that Circuit.from_ir correctly handles verbatim_box sections.
The test was ran via command:
pytest test/unit_tests/braket/circuits/test_circuit.py::test_from_ir_with_verbatim_box
and
pytest test/unit_tests/braket/circuits/test_circuit.py::test_from_ir_with_verbatim_non_verbatim_instr
They both pass:
test/unit_tests/braket/circuits/test_circuit.py::test_from_ir_with_mixed_verbatim_non_verbatim_instr
[gw11] [100%] PASSED test/unit_tests/braket/circuits/test_circuit.py::test_from_ir_with_mixed_verbatim_non_verbatim_instr
====================== 1 passed in 5.89s =====================
test/unit_tests/braket/circuits/test_circuit.py::test_from_ir_with_verbatim_box
[gw11] [100%] PASSED test/unit_tests/braket/circuits/test_circuit.py::test_from_ir_with_verbatim_box
====================== 1 passed in 6.11s ======================
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
- [ x] I have read the CONTRIBUTING doc
- [ x] I used the PR title format described in CONTRIBUTING
- [ x] I have updated any necessary documentation, including READMEs and API docs (if appropriate)
Tests
- [ x] I have added tests that prove my fix is effective or that my feature works (if appropriate)
- [ x] 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.
I am currently completing the changes in my fork of amazon-braket-default-simulator-python. I will then open a PR, which should be reviewed alongside the one above. Thank you.
I have also successfully performed testing via tox -e unit-tests for amazon-braket-sdk-python
========== 2853 passed, 129 xfailed, 8 warnings in 141.91s (0:02:21) ===========
unit-tests: OK (187.87=setup[41.88]+cmd[145.99] seconds)
congratulations :) (187.96 seconds)
hello @rmshaffer I will fix the code format issues
The test will not pass until PR here is also confirmed and merged to main. The changes in the sdk work only if the amazon-default-simulator is updated
The test will not pass until PR here is also confirmed and merged to main. The changes in the sdk work only if the amazon-default-simulator is updated
Got it, thanks! If you want to try to pass the CI in the meantime, before that PR is merged, I believe you could temporarily update this line in this PR: https://github.com/robotastray/amazon-braket-sdk-python/blob/main/tox.ini#L143 to point to your fork, i.e.
git+https://github.com/robotastray/amazon-braket-default-simulator-python.git
hello @rmshaffer I will fix the code format issues
Yes, I see a lot of extraneous formatting changes in this PR - let us know if you need assistance with this.
You should just be able to run ruff check --fix from within the src folder to clean everything up there.
In the test folder, ruff is not actually run in CI. So you'll probably want to revert all of the formatting-specific changes that ruff made there, so that your PR doesn't have so many extra changes.
The test will not pass until PR here is also confirmed and merged to main. The changes in the sdk work only if the amazon-default-simulator is updated
Got it, thanks! If you want to try to pass the CI in the meantime, before that PR is merged, I believe you could temporarily update this line in this PR: https://github.com/robotastray/amazon-braket-sdk-python/blob/main/tox.ini#L143 to point to your fork, i.e.
git+https://github.com/robotastray/amazon-braket-default-simulator-python.git
I have made the changes as you suggested
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 100.00%. Comparing base (fa6f520) to head (17f1143).
:warning: Report is 75 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #1085 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 146 146
Lines 9872 9877 +5
Branches 1164 1164
=========================================
+ Hits 9872 9877 +5
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Codecov Report
Attention: Patch coverage is
88.88889%with1 linein your changes missing coverage. Please review.Project coverage is 99.98%. Comparing base (
1b71ac8) to head (d9d6ddf). Report is 3 commits behind head on main.Files with missing lines Patch % Lines src/braket/circuits/braket_program_context.py 88.88% 0 Missing and 1 partial ⚠️ Additional details and impacted files
☔ View full report in Codecov by Sentry. 📢 Have feedback on the report? Share it here. 🚀 New features to boost your workflow:
@rmshaffer is this failure about testing not being covered for the line of code highlighted?
@robotastray it's indicating that both branches of the highlighted line are not being hit. To fix that error, you would want tests which cover all branching cases: enter the if block, enter the elif block, and skip both blocks.
I have added additional testing and I have also updated the function add_verbatim_marker to make sure that if an invalid marker is added a TypeError is raised.
I am currently looking at test/unit_tests/braket/experimental_capabilities/test_experimental_capabilities.py to see how my changes have impacted it. I will update once I have more info
@rmshaffer the automated tests are checking for test_measureff_ccprx_from_ir. the function does not exists in test/unit_tests/braket/experimental_capabilities/iqm/test_iqm_experimental_capabilities.py
you can see the details in line 6376 on this page
@rmshaffer the automated tests are checking for
test_measureff_ccprx_from_ir. the function does not exists intest/unit_tests/braket/experimental_capabilities/iqm/test_iqm_experimental_capabilities.pyyou can see the details in line 6376 on this page
@robotastray that test can be found in your fork here: https://github.com/robotastray/amazon-braket-sdk-python/blob/main/test/unit_tests/braket/experimental_capabilities/iqm/test_iqm_experimental_capabilities.py#L190-L206
It's a recently-added test in the upstream repo. I think this failure is expected - your changes are now parsing additional instructions out of the IR. I think you can just update the assertions there so that the test passes again with your changes. For example, replace
assert len(circuit_from_ir.instructions) == 3
with
assert len(circuit_from_ir.instructions) == 5
since it now contains the "start" and "end" verbatim box instructions.
I updated test_iqm_experimental_capabilities.py to take into account the verbatim box. In addition to assert len(circuit_from_ir.instructions) == 5
I have also added few lines and updated others
assert circuit_from_ir.instructions[0].operator.name == "StartVerbatimBox"
assert circuit_from_ir.instructions[1].operator.name == "PRx"
assert isinstance(circuit_from_ir.instructions[2].operator, MeasureFF)
assert isinstance(circuit_from_ir.instructions[3].operator, CCPRx)
assert circuit_from_ir.instructions[4].operator.name == "EndVerbatimBox"
finally the indces have been updated on lines 200 and 205 to reflect the changes above
@rmshaffer I updated the code in test_circuit.py and braket_program_context.py to make sure it is inline with the updates in #285
Great work!