Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@swernli
Copy link
Collaborator

@swernli swernli commented Aug 20, 2020

This change splits all the intrinsics and decompositions defined in QsharpCore out into their own files to allow them to be individually combined into a target definition package.

Q# should include decompositions to alternate quantum gate sets #249

This change splits all the intrinsics and decompositions defined in QsharpCore out into their own files to allow them to be individually combined into a target definition package.

 Q# should include decompositions to alternate quantum gate sets #249
@swernli
Copy link
Collaborator Author

swernli commented Aug 20, 2020

This PR is just to make viewing the incremental changes easier. We can have a larger review when this eventually makes it from my swernli/decomp-working branch to the feature/decomp branch (after resolving the current pending merge of master there).

@@ -0,0 +1,39 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alan-geller FYI, this is what target definition "package" will look like. This allows any project to pull in the API surface defined for this target by just importing this props file in the project. Let me know if you have any questions or concerns regarding this approach!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

I can see how for simulators like Toffoli or CHP (or the full state simulator, of course), and for the hardware targets as well, it's very clean to get exactly the operations you want. It also avoids the need to repeat the decompositions for each simulator or hardware target they apply to.

I assume that if I have a simulator (or hardware target, for that matter) that implements some additional operation as an intrinsic, I would just add an additional file to the Intrinsic directory and include that rather than the Decompositions version? So, for instance, if I had an intrinsic SWAP, I'd create a SWAP.qs in the Intrinsic directory and include that in my project instead of the one from the Decompositions folder?

I guess I could also create a TargetDefinition.qs file in my project where I include any "overrides" like that. Which do you think will be better?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding how to extend this, I was imagining the first option you describe: create a new SWAP.qs in the Intrinsic folder and link against that one instead of the decomposition in your props file. Right now this is all very clean and easy, where the names of the files match the names of the callable within, but that won't last forever. For example, if two targets have different intrinsics, they might need different implementations of the same decomposed callable, so you could end up with something like SWAPfromCNOT.qs vs SWAPfromControlledR.qs where the name is meant to be succinctly but uniquely descriptive of what's inside even though both contain an implementation of the same callable, namely Microsoft.Quantum.Intrinsic.SWAP.

I'll be getting into this very shortly with my next working branch PR and we can continue to discuss the approach there.

@swernli swernli requested a review from cgranade August 20, 2020 19:55
@swernli swernli merged commit f698172 into swernli/decomp-working Aug 20, 2020
Adjoint _flipToBasis(stateId, qubits);
AssertMeasurementProbability(observable, qubits, Zero, expectedZeroProbability, $"", Accuracy());
AssertMeasurementProbability(observable, qubits, One, expectedOneProbability, $"", Accuracy());
Adjoint flipToBasis(stateId, qubits);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you do this with a within/apply construct? within flipToBasis apply the two assserts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very likely! I didn't look too hard at the implementations from the perspective of refactoring to use the latest syntax, but I think that's something we should definitely do at somepoint.

@@ -0,0 +1,39 @@
<?xml version="1.0" encoding="utf-8"?>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

I can see how for simulators like Toffoli or CHP (or the full state simulator, of course), and for the hardware targets as well, it's very clean to get exactly the operations you want. It also avoids the need to repeat the decompositions for each simulator or hardware target they apply to.

I assume that if I have a simulator (or hardware target, for that matter) that implements some additional operation as an intrinsic, I would just add an additional file to the Intrinsic directory and include that rather than the Decompositions version? So, for instance, if I had an intrinsic SWAP, I'd create a SWAP.qs in the Intrinsic directory and include that in my project instead of the one from the Decompositions folder?

I guess I could also create a TargetDefinition.qs file in my project where I include any "overrides" like that. Which do you think will be better?

swernli added a commit that referenced this pull request Aug 25, 2020
This change splits all the intrinsics and decompositions defined in QsharpCore out into their own files to allow them to be individually combined into a target definition package.

 Q# should include decompositions to alternate quantum gate sets #249
@swernli swernli deleted the swernli/qsharpcore-targetdef branch September 4, 2020 05:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants