Add circuit loader methods: from_ir, from_repr and from_diagram
Issue #, if available:
Description of changes: Add three circuit loader methods to the Circuit class:
-
from_ir(program: Program) -> Circuit- Create Circuit from Program object. -
from_repr(repr_str: str) -> Circuit- Create Circuit from a string produced by repr(circuit). -
from_diagram(diagram_str: str) -> Circuit- Create Circuit from a circuit diagram string produced by f'{circuit}'.
These methods are added to the Circuit class as a subroutine using @circuit.subroutine(register=True).
Testing done:
-
./test/unit_tests/braket/circuits/test_from.pyto test 30 test cases of circuit ascii diagrams covering quantum gates, noise operators and result types. (The ascii diagram files are stored under from/input/test_cases.txt.) -
./test/unit_tests/braket/circuits/test_from_matrics.pyto test the unitary gate, the Kraus noise operator and the Hermitian result type. -
./test/unit_tests/braket/circuits/test_from_xfail.pyto test error cases.
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 commit message 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.
Thanks for the PR @kjacky! This would be a very useful addition. Following are some initial thoughts:
-
It'd be nice to break out the PR to focus on a single type of translation. This would help make it easier to review the changes. I'd recommend starting with
from_ir()for the following reasons:- The IR contains the complete representation of the circuit. Other representations would truncate information (such as the exact value for an angle in an angled gate) or mask it (Unitary gate is represented as "Unitary" in an ASCII diagram and doesn't contain the information about the unitary matrix being used). This would cause the
from_reprandfrom_diagramto not work in all cases. - The IR can always be retrieved post-execution (from the task execution results). The circuit repr and diagram are not stored by default. Hence, a customer won't be able to use the
from_reprandfrom_diagramunless they explicitly store these, and if they already have a way to reconstruct the circuit from the stored representation (IR) it's questionable what value these methods would add. - Methods such as
from_diagramcan be very brittle since they rely on correctly parsing extraneous formatting of the circuit layout. The circuit formatting can change frequently causing these methods to break.
- The IR contains the complete representation of the circuit. Other representations would truncate information (such as the exact value for an angle in an angled gate) or mask it (Unitary gate is represented as "Unitary" in an ASCII diagram and doesn't contain the information about the unitary matrix being used). This would cause the
-
The current
from_irimplementation is too complex (which you might have seen the build failure complaining about as well). We should try to think about how to simplify this. Here are a couple of ideas which might be worth considering:- The current implementation aggregates all the logic across multiple object types in a single method. This would be hard to maintain and reason about as we add in new object types. One way to solve this would be to delegate the responsibility to the different object types themselves. You might have seen that each class (Circuit, Instruction, Gate, ResultType) have a
to_ir()method which contains logic on how that specific object can be converted to the IR representation, and the classes delegate this conversion where possible (Circuit delegates the IR conversion for instructions to the Instruction class, which in turn delegates the conversion for the quantum operator to the specific operator class). Similarly, each class could have afrom_ir()method which knows how to construct an instance of the specific class from the IR representation.
class Circuit: @classmethod def from_ir(cls, program: Program) -> Circuit: instructions = [Instruction.from_ir(program.instructions)] results = [ResultType.from_ir(program.results)] return cls(instructions + results)- We are creating some duplicated mappings
obj_class_defs,gate_defs,result_type_defs,observable_defs,gate_to_observable. Consider how we could reuse the existing mappings to serve this use case. It might help to look at how some of the classes are storing this information using theregistermethods. (See Gate, Noise, ResultType)
- The current implementation aggregates all the logic across multiple object types in a single method. This would be hard to maintain and reason about as we add in new object types. One way to solve this would be to delegate the responsibility to the different object types themselves. You might have seen that each class (Circuit, Instruction, Gate, ResultType) have a
Codecov Report
Merging #256 (f5d810e) into main (9c4f94d) will not change coverage. The diff coverage is
100.00%.
@@ Coverage Diff @@
## main #256 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 41 42 +1
Lines 3017 3140 +123
Branches 414 424 +10
==========================================
+ Hits 3017 3140 +123
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/braket/circuits/observables.py | 100.00% <ø> (ø) |
|
| src/braket/circuits/angled_gate.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/circuit.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/circuit_utils.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/gate.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/gates.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/instruction.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/noise.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/noises.py | 100.00% <100.00%> (ø) |
|
| src/braket/circuits/result_type.py | 100.00% <100.00%> (ø) |
|
| ... and 2 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 9c4f94d...f5d810e. Read the comment docs.
Thank you very much @kshitijc for your guidance. I have removed from_diagram and from_repr, then refactored from_ir into individual braket.circuits classes. All *_defs and conversions were removed.
Each of the 16 from_ir() class methods returns an object of the cls type. All are 1-to-3 liners, except for Instruction::from_ir() with 15 non-comment lines implementing the core logic. Tests and coverage were passed 100%.
I've added two utility functions for internal use: complex_matrices() and _attr_dict() (near the bottom of circuits/result_type.py for lack of a better place I found - feel free to move them).
Looking forward to your feedback @kshitijc .
Closing PR since there has been no activity on this for a while. Please feel free to reopen if you wish to continue working on this. Thanks!