[Quake] Implementation, canonicalization, and lowering of quake.phase, an op designed to account for global/controlled phase - #4976
Conversation
CI Summary (
|
| Job | Result |
|---|---|
binaries |
⏩ skipped |
build_and_test |
✅ success |
config_devdeps |
✅ success |
config_source_build |
⏩ skipped |
config_wheeldeps |
✅ success |
devdeps |
✅ success |
docker_image |
⏩ skipped |
gen_code_coverage |
⏩ skipped |
metadata |
✅ success |
python_metapackages |
⏩ skipped |
python_wheels |
⏩ skipped |
source_build |
⏩ skipped |
wheeldeps |
✅ success |
⏩ Skipped jobs (7) — intentionally skipped on PR builds; run on merge_group / workflow_dispatch
| Job |
|---|
binaries |
config_source_build |
docker_image |
gen_code_coverage |
python_metapackages |
python_wheels |
source_build |
All sub-jobs (42) — every matrix leg, with links
| Job | Status | Link |
|---|---|---|
| Build and test (amd64, gcc12, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, gcc12, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (amd64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| CI Summary | ❔ in_progress | view |
| Configure build (devdeps) | ✅ success | view |
| Configure build (source_build) | ⏩ skipped | view |
| Configure build (wheeldeps) | ✅ success | view |
| Create CUDA Quantum installer | ⏩ skipped | view |
| Create Docker images | ⏩ skipped | view |
| Create Python metapackages | ⏩ skipped | view |
| Create Python wheels | ⏩ skipped | view |
| Gen code coverage | ⏩ skipped | view |
| Load dependencies (amd64, gcc12) / Caching | ✅ success | view |
| Load dependencies (amd64, gcc12) / Finalize | ✅ success | view |
| Load dependencies (amd64, gcc12) / Metadata | ✅ success | view |
| Load dependencies (amd64, llvm) / Caching | ✅ success | view |
| Load dependencies (amd64, llvm) / Finalize | ✅ success | view |
| Load dependencies (amd64, llvm) / Metadata | ✅ success | view |
| Load dependencies (arm64, gcc12) / Caching | ✅ success | view |
| Load dependencies (arm64, gcc12) / Finalize | ✅ success | view |
| Load dependencies (arm64, gcc12) / Metadata | ✅ success | view |
| Load dependencies (arm64, llvm) / Caching | ✅ success | view |
| Load dependencies (arm64, llvm) / Finalize | ✅ success | view |
| Load dependencies (arm64, llvm) / Metadata | ✅ success | view |
| Load source build cache | ⏩ skipped | view |
| Load wheel dependencies (amd64, 12.6) / Caching | ✅ success | view |
| Load wheel dependencies (amd64, 12.6) / Finalize | ✅ success | view |
| Load wheel dependencies (amd64, 12.6) / Metadata | ✅ success | view |
| Load wheel dependencies (amd64, 13.0) / Caching | ✅ success | view |
| Load wheel dependencies (amd64, 13.0) / Finalize | ✅ success | view |
| Load wheel dependencies (amd64, 13.0) / Metadata | ✅ success | view |
| Load wheel dependencies (arm64, 12.6) / Caching | ✅ success | view |
| Load wheel dependencies (arm64, 12.6) / Finalize | ✅ success | view |
| Load wheel dependencies (arm64, 12.6) / Metadata | ✅ success | view |
| Load wheel dependencies (arm64, 13.0) / Caching | ✅ success | view |
| Load wheel dependencies (arm64, 13.0) / Finalize | ✅ success | view |
| Load wheel dependencies (arm64, 13.0) / Metadata | ✅ success | view |
| Prepare cache clean-up | ✅ success | view |
| Retrieve PR info | ✅ success | view |
✅ Required checks (6/6) — declared in .github/required-checks.yml for push
| Required check | Status | Link |
|---|---|---|
| Build and test (amd64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (amd64, gcc12, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, gcc12, openmpi) / Dev environment (Python) | ✅ success | view |
23e551f to
c1bebd1
Compare
c1bebd1 to
6c3d6f0
Compare
6c3d6f0 to
40bb5d7
Compare
atgeller
left a comment
There was a problem hiding this comment.
Some questions based on a quick look.
40bb5d7 to
9f868b5
Compare
5b08fad to
3aea31d
Compare
3aea31d to
1d0572c
Compare
sacpis
left a comment
There was a problem hiding this comment.
Thanks @cabreraam. Left few comments.
| phase, phase.getResultTypes(), adjoint, ValueRange{mergedAngle}, | ||
| input.getControls(), input.getTargets(), | ||
| input.getNegatedQubitControlsAttr()); | ||
| rewriter.eraseOp(input); |
There was a problem hiding this comment.
MergeAdjacentPhasePattern calls rewriter.eraseOp(input) unconditionally. It only checks that the second phase consumes the first phase's results, never that those results have no other users. Seems like verifyWireResultsAreLinear tolerates a wire result having several users when each is in a distinct mode, so such IR is legal input.
Tried running this mlir (xyz.qke)
func.func @multiuse(%a: f64, %b: f64, %cond: i1) {
%c = quake.null_wire
%an = quake.null_wire
%p1:2 = quake.phase (%a) [%c] %an : (f64, !quake.wire, !quake.wire) -> (!quake.wire, !quake.wire)
%p2:2 = quake.phase (%b) [%p1#0] %p1#1 : (f64, !quake.wire, !quake.wire) -> (!quake.wire, !quake.wire)
cf.cond_br %cond, ^bb1, ^bb2
^bb1:
quake.sink %p1#0 : !quake.wire
quake.sink %p2#0 : !quake.wire
quake.sink %p2#1 : !quake.wire
return
^bb2:
quake.sink %p2#0 : !quake.wire
quake.sink %p2#1 : !quake.wire
return
}
with this command
/cudaq-opt --canonicalize xyz.qke
and got this error
cudaq-opt: /home/.llvm-project/mlir/lib/IR/PatternMatch.cpp:156: virtual void mlir::RewriterBase::eraseOp(mlir::Operation*): Assertion `op->use_empty() && "expected 'op' to have no uses"' failed.
PLEASE submit a bug report to https://github.com/NVIDIA/cuda-quantum and include the crash backtrace.
| if (auto quantum = dyn_cast<cudaq::quake::OperatorInterface>(cursor)) | ||
| if (failed(advanceAcrossOperator(quantum, controls, anchor))) { | ||
| destination = cursor; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Seems like normalize-phase-placement emits use-before-def IR when one control can move past a gate but another cannot. Can you please try running a 2 control wire phase, one safe gate, then an x that targets the second control?
| quake.sink %second#0 : !quake.wire | ||
| quake.sink %second#1 : !quake.wire | ||
| return | ||
| } |
There was a problem hiding this comment.
Are we missing tests covering the adjoint sign math in the phase merges?
|
|
||
| namespace { | ||
|
|
||
| static SmallVector<Type> getWireResultTypes(MLIRContext *context, |
There was a problem hiding this comment.
Seems like this exists in NormalizePhasePlacement as well.
| if (phase.isAdj()) | ||
| angle = arith::NegFOp::create(rewriter, phase.getLoc(), angle); | ||
| return angle; | ||
| } |
There was a problem hiding this comment.
Seems like similar to getSignedAngle in NormalizePhasePlacement?
| assert(replacements.size() == phase.getWires().size() && | ||
| "phase result count does not match its wire operands"); | ||
| return replacements; | ||
| } |
There was a problem hiding this comment.
Seems like heavily overlapped with getWireInputs in NormalizePhasePlacement?
There was a problem hiding this comment.
Should we move similar methods to some utility file?
| break; | ||
| } | ||
|
|
||
| crossedOperation = true; |
There was a problem hiding this comment.
Should we only set crossedOperation when the crossed op is a quake::OperatorInterface?
| def LowerPhase : Pass<"lower-phase", "mlir::func::FuncOp"> { | ||
| let summary = "Lower phase bookkeeping operations to ordinary Quake gates"; | ||
| let description = [{ | ||
| Lowers controlled `quake.phase` operations to exact `quake.r1``quake.rz` |
There was a problem hiding this comment.
nit: missing separator (/) between quake.r1``quake.rz?
1d0572c to
3a88202
Compare
…e`, an op designed to account for global/controlled phase Signed-off-by: Anthony Cabrera <antcabrera@nvidia.com>
Signed-off-by: Anthony Cabrera <antcabrera@nvidia.com>
…entries in the negated control boolean array Signed-off-by: Anthony Cabrera <antcabrera@nvidia.com>
…common file `PhaseUtilities`; rename `hasUnambiguousLinearUse` --> `hasUnambiguousWireUse` Signed-off-by: Anthony Cabrera <antcabrera@nvidia.com>
Signed-off-by: Anthony Cabrera <antcabrera@nvidia.com>
3a88202 to
0246116
Compare
Overview
This PR introduces the
quake.phaseop, which accounts for phase corrections, both global and controlled, its canonicalization, and lowering. For lowering, we introduce two passes,NormalizePhasePlacementfor normalizing the location of phase ops in the IR, andLowerPhase, which either erases uncontrolledphaseops or introduces the correspondingr1/rzrotations that map to the givenphaseop.The size of this PR is a bit of a doozy (though the line count is inflated by the size of the tests), so in the following sections, I've outlined each implementation detail and the corresponding code.
Implementation
quake.phaseopQuakeOps.td, see the tablegen entry forPhaseOpOneTargetParamOpwhere phase angle is the one parameterVerification
QuakeOps.cppquake.refofquake.wiretypephase-op-errors.qkeMatrix Implementation
PhaseOp::getOperatorMatrixinQuakeOps.cppphase-matrix.qkeuses this matrix representation to testquake.phaseops against their lowered counterpart for equivalenceMemory Effects
Canonicalization
Two patterns registered in
PhaseOp::getCanonicalizationPatternsinQuakeOps.cppImplemented in
CanonicalPatterns.incTested in
phase-canonicalize.qkeFolding angles congruent to$0 \bmod 2\pi$
rewrite pattern that attempts to erase a$2/pi$
quake.phaseop if it`s angle is a multiple ofrejects non-finite values
compares how close the angle is to identify phase. If the angle is a multiple of$2\pi$ , then the following is true
I represent$2\pi$ by creating an $2\pi$ , so if this is not the best way to go about this, I'm happy to go back to the drawing board/remove this canonicalization
llvm::APFloatwith a string representation ofAdjacent Phase Merging
phaseops, e.g.,NormalizePhasePlacement, but having this canonicalization pattern present allows us to merge any low hanging fruit whencanonicalizeis runNormalizePhasePlacementCreates a transformation pass invoked with
normalize-phase-placementthat attempts to sink phase operations as far down in their respective basic blocks as is safe, e.g., not crossing calls, regions, terminators, non-unitary quantum ops, and ops with unknown effectsImplemented in
NormalizePhasePlacement.cppand tested innormalize-phase-boundaries.qkeLower Phase
Erases all uncontrolled$\ket{1}$ state.
quake.phaseops and lowers all controlledquake.phaseops intor1rotations (rotations about theImplemented in
LowerPhase.cppand tested inlower-phase.qke.Adjoint Handling
Negates the phase angle
Scalar-control lowering
When a control is a scalar, that control becomes the
quake.phaseanchor.When there are multiple scalar controls, either
is selected as the anchor qubit with the other remaining as controls. The resulting
r1op then becomes a rotation on the anchor qubit controlled by the remaining controls.Aggregate-only fallback
If the type of the controls is a container of unknown size i.e.,
veqwith dynamic size, we can use the equationto handle this fallback condition where, even though we don't explicitly know the controls, we can just implement controlled versions of the
r1andrzrotations to achieve the same behavior as if the controls were known and scalar(s), e.g.,The caveat is if
Anchor alias checking
There is a check to make sure that the anchor qubit is not part of the aggregate control container. This situation shouldn't happen, so if it does, we emit an error.
Wire result restoration
When dealing with
wiretypes, we need to map the outputs of the loweredphaseop(s) to their correct destinations. For example, in the testWe lower the
phaseop towhere the the
controlandanchorvalues map to%2and%1, respectively.