Add mqbc_gate_set_pass and e2e test file #8462
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
lillian542
changed the title
lillian542
changed the title
There was a problem hiding this comment.
Thanks @lillian542 ! LGTM and just a few comments for your consideration.
| from .decomposition import convert_to_mbqc_formalism, convert_to_mbqc_gateset | ||
| from .decomposition import ( | ||
| convert_to_mbqc_formalism, | ||
| convert_to_mbqc_gateset, |
There was a problem hiding this comment.
Do we still need convert_to_mbqc_gateset?
There was a problem hiding this comment.
I would prefer to leave it. It's separate from convert_to_mqbc_gateset_pass in that it's been decorated with transform, so it's a proper TransformDispatcher and therefore more "by the book" for a tape transform - and we also never call tape transforms "passes".
But technically convert_to_mbqc_gateset_pass should work everywhere I can think of where convert_to_mbqc_gateset works, so I can see an argument against keeping it.
|
|
||
| def e2e_mbqc_pipeline(qnode): | ||
| """All the transforms currently in the E2E pipeline for MBQC test workload""" | ||
| # qnode = outline_state_evolution_pass(qnode) |
There was a problem hiding this comment.
Could we put all passes into a list and then iterate and apply for the list?
| # measurements have been updated | ||
| # CHECK-NOT: quantum.namedobs | ||
| # CHECK: quantum.sample | ||
| for i in range(x): |
There was a problem hiding this comment.
so for loops can be captured? I remember we couldn't.
There was a problem hiding this comment.
I think you are thinking of the issue where autograph wasn't working with the unified compiler, but that's been resolved
| # qnode = split_non_commuting_pass(qnode) | ||
| qnode = diagonalize_final_measurements_pass(qnode) | ||
| qnode = measurements_from_samples_pass(qnode) | ||
| qnode = convert_to_mbqc_gateset_pass(qnode) |
There was a problem hiding this comment.
Wondering if the current order of applying transforms would ensure convert_to_mbqc_gateset_pass is compatible with outline_state_evolution_pass. With the outline_state_evolution_pass, all gates are moved to a func.func without qnode attribute.
There was a problem hiding this comment.
I think that's a great question that we will be able to answer when we uncomment it to add it to the integration tests here 😅 That's exactly the kind of issue I'm hoping these tests will make apparent.
If it's not compatible, I would say that's an issue that needs to be resolved by modifying one of the transforms. We've been treating this as the canonical order of these transforms for quite some time. But is there an alternative order you had in mind?
| # CHECK: quantum.sample | ||
| qml.RZ(x, 0) | ||
| qml.RZ(y, 1) | ||
| qml.RX(x, 0) |
There was a problem hiding this comment.
Maybe we can add a TODOs for multirz and basisrotation gates here.
There was a problem hiding this comment.
Good point! MultiRZ is included in the "pretend_xas_workflow" test below (I'm hoping a fix will merge for that soon but if this is ready to merge before then I'll xfail the test for now), but I'll add BasisRotation to that as well. And maybe MPSPRep.
|
thanks @lillian542 , I would like to have a final round of review after the CI failures are fixed. |
Context:
convert_to_mbqc_gateset_passwrapper function so we don't have to have the RotXZX decomposition and the gateset in every notebook for playing with the MBQC pipelineDescription of the Change:
These two things are added.
Benefits:
Convenience.
[sc-93172][sc-100901]