Add the XY, PISWAP and CAN standard gate matrices - #521
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
The Quil specification defines XY, PISWAP, and CAN among its standard gates, and `ReservedGate` already lists all three, but `to_unitary` had no matrices for them: XY and PISWAP were rejected as undefined gates, and CAN, which takes three parameters, could not be expressed at all. Parameterized gate matrices now carry the number of parameters they take, so CAN can sit alongside the one-parameter gates, and a standard gate applied to the wrong number of parameters reports how many that gate takes. A parameterized gate that isn't a standard gate is now reported as undefined rather than as having too many parameters.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR proposes adding the
XY,PISWAPandCANstandard gate matrices soGate::to_unitarycan build all three, and reporting a parameterized gate that isn't a standard gate as undefined instead of as one given too many parameters (fixes #465). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/256. You can sign in with your GitHub ID to claim ownership of the project.What was wrong
§4.3 of the Quil specification defines
XY,PISWAPandCANamong the standard gates, andReservedGateinquil-rs/src/reserved.rsalready lists all three — butPARAMETERIZED_GATE_MATRICEShad no entry for any of them, so neitherGate::to_unitarynorProgram::to_unitarycould produce their matrices. Running each ofProgram::from_str(source).unwrap().to_unitary(2)onmainat ed186cb:The last two lines are the same limitation, not two different ones: the matrix table could only describe one-parameter gates, so
expected 1 parametersis that ceiling talking rather than anything aboutCANorFSIM.The change
ParameterizedMatrixbecomes a small enum whose variants carry how many parameters a gate takes (OneParameter,ThreeParameters).gate_matrixnow evaluates all of a gate's parameters, looks the gate up once, and asks it for its matrix, soCANsits in the same table as the one-parameter gates,MatrixArgumentLengthreports the count the gate actually takes, and a name that isn't a standard gate comes back asUndefinedGateregardless of how many parameters it was given — which is the message #466 asks for onFSIM.The matrices are transcribed from the gate definitions in §4.3, which agree entry-for-entry with
quilc'ssrc/quil/stdgates.quil, so this adds no new convention:XY(theta)andPISWAP(theta)are the same operator there and share one implementation here, andCAN(alpha, beta, gamma)is the closed form given in the specification. Nothing else about the standard gate table moves, so existing callers see identical matrices for every gate that already worked.Verification
Same four programs, after the change:
Eighteen test cases came with it, in
test_gate_into_matrix:test_can_matches_specificationevaluates the specification's ownDEFGATE CAN(%alpha, %beta, %gamma)expressions throughExpression::from_strandevaluate, and compares that matrix againstto_unitary, over five parameter triples — so the entry is pinned to the specification text rather than to my transcription of it.test_xy_and_piswap_matriceschecks both names against the specification matrix at four angles,test_new_gates_are_unitarychecksU†U = I, and two newtest_to_unitarycases liftXYandCANinto a three-qubit space.test_to_unitary_parameter_errorscovers the error paths: an unknown parameterized gate, a standard gate given the wrong number of parameters (both one- and three-parameter), and a non-constant parameter.Removing just the three new table entries turns 15 of those cases red, so they do test the change rather than the harness.
cargo test -p quil-rsgoes from 2976 passing / 0 failing on the unmodified tree to 2994 passing / 0 failing with the change;cargo test -p quil-cli,cargo clippy --workspace --all-targets --all-features -- -D warningsandcargo fmt --all --checkare clean before and after.One thing I noticed while checking the table against §4.3 and deliberately kept out of this PR: the
RZentry is theRYmatrix rather thandiag(cis(-theta/2), cis(theta/2)), andPSWAP's off-diagonal entry istheta.cos() + thetarather thancis(theta). Both look like transcription slips, and unlike the change here they would move results that callers already get, so they seem worth their own PR rather than a rider on this one — happy to send that if it is useful.How this was managed
This work was tracked as a story on a board imported from this repository's own issues and pull requests — 413 stories in all — which you can browse at https://eastagiletracker.com/projects/256.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com