Added QASM Import Support for rzz, rxx, ryy, crx, cry, and iswap Gates #7290
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7290 +/- ##
==========================================
- Coverage 98.67% 98.66% -0.01%
==========================================
Files 1106 1106
Lines 96142 96219 +77
==========================================
+ Hits 94864 94939 +75
- Misses 1278 1280 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I added a test to check if the unitaries are the same and the test is passing. Surprisingly they are exactly the same, but I added some extra tolerance since I'm not 100% sure if that's expected behavior |
There was a problem hiding this comment.
I'll go ahead and give it an "lgtm", though I don't know the unitaries of the other new gates either, so I'm assuming you've verified that they're all what they should be. If you could add similar unitary equivalence tests before merging, that would improve the PR significantly (and that may be required by final review). If you can figure out a way to make a parameterized test that does this for all the gates in qelib_gates automatically, even better.
cc @pavoljuhas for final review.
|
Sounds good, I'll make sure to add that |
|
New unit tests are reformatted to be more aligned with how other gates are tested and there is 1 test to test all unitary equivalence |
| ) | ||
| def test_rxx_unitary_equivalence(theta_expr, theta): | ||
| def test_all_qelib_gates_unitary_equivalence(qasm_gate, cirq_gate, num_params, num_args): | ||
| theta = np.pi / 4 |
There was a problem hiding this comment.
Very cool! The only thing I'd say is probably use different random numbers for the parameters, and maybe loop through a couple times with different parameters to make sure it's not just getting lucky and testing a parameter that happens to work.
There was a problem hiding this comment.
Is there a benefit to testing all gates individually? Can probably use this same logic to test all gate equivalences as well
Description
Fixes the code in #7007
Builds on #7072
Newly Supported Gates:
rzz: Imported as cirq.ZZPowGate
rxx: Imported as cirq.XXPowGate
ryy: Imported as cirq.YYPowGate
crx: Imported as cirq.ControlledGate(cirq.Rx)
cry: Imported as cirq.ControlledGate(cirq.Ry)
iswap: Imported as cirq.ISwapPowGate
Added unit tests for these gates, and turned gates into alphabetical order for future additions