From f27b4a14f27447b7d4e09f6581b1b1f490a15be8 Mon Sep 17 00:00:00 2001 From: Mathias Soeken Date: Thu, 5 May 2022 13:19:31 +0200 Subject: [PATCH 1/4] API review May 2022 --- Design/meetings/2022/api-design-2022-05.md | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 Design/meetings/2022/api-design-2022-05.md diff --git a/Design/meetings/2022/api-design-2022-05.md b/Design/meetings/2022/api-design-2022-05.md new file mode 100644 index 00000000000..178392fa380 --- /dev/null +++ b/Design/meetings/2022/api-design-2022-05.md @@ -0,0 +1,45 @@ +# Q# API Design Discussions / May 2022 + +Reviewers (in order by username): ... + +## Agenda + +- https://github.com/microsoft/QuantumLibraries/issues/579 +- https://github.com/microsoft/qsharp-runtime/issues/1005 +- https://github.com/microsoft/QuantumLibraries/issues/423 + +## Discussion + +### Move Exp into libraries instead of intrinsics + +**Proposal**: https://github.com/microsoft/QuantumLibraries/issues/579 + +**Reviews**: + +> Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. + +**Consensus**: + +--- + +### Add MeasureEachZ operation to runtime implementation + +**Proposal**: https://github.com/microsoft/qsharp-runtime/issues/1005 + +**Reviews**: + +> Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. + +**Consensus**: + +--- + +### Simple and consistent arithmetic API + +**Proposal**: https://github.com/microsoft/QuantumLibraries/issues/423 + +**Reviews**: + +> Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. + +**Consensus**: From f248c66f35c23a00030c376d1ebc6223176780d3 Mon Sep 17 00:00:00 2001 From: Mathias Soeken Date: Thu, 5 May 2022 22:36:19 -0700 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Cassandra Granade --- Design/meetings/2022/api-design-2022-05.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Design/meetings/2022/api-design-2022-05.md b/Design/meetings/2022/api-design-2022-05.md index 178392fa380..c2de434cba1 100644 --- a/Design/meetings/2022/api-design-2022-05.md +++ b/Design/meetings/2022/api-design-2022-05.md @@ -15,6 +15,8 @@ Reviewers (in order by username): ... **Proposal**: https://github.com/microsoft/QuantumLibraries/issues/579 **Reviews**: +- **Approved with suggestions.** I think this makes a lot of sense; Exp is extremely useful, but it's much higher-level than any other intrinsic, making it an odd fit for the Microsoft.Quantum.Intrinsics namespace. If I understand correctly, we did so due to the full-state simulator having some nice shortcuts for simulating `Exp`, but I don't think that implementation detail justifies including `Exp` in the basic set of intrinsics. + I would suggest, however, that if we move `Exp` and make a breaking change that way, we also take the opportunity to give it a more clear name that reflects its action a bit better. In particular, `Exp` simulates evolution under a Pauli Hamiltonian such that `Microsoft.Quantum.Simulation.EvolveUnderPauli` may make sense; alternatively, it can be thought of as applying a joint rotation, perhaps justifying `Microsoft.Quantum.Canon.ApplyJointRotation`? > Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. @@ -27,7 +29,7 @@ Reviewers (in order by username): ... **Proposal**: https://github.com/microsoft/qsharp-runtime/issues/1005 **Reviews**: - +- **Approved.** I think this is a pretty straightforward one, and works great for replacing the old `MultiM` with something more maintainable and easier to use. > Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. **Consensus**: @@ -40,6 +42,11 @@ Reviewers (in order by username): ... **Reviews**: +- **Comment.** + - I'm a bit concerned that names like `GreaterThan` read like functions (noun or adjective) rather than as operations. + - How does it look to chain carry-out qubits to the `carryIn` of subsequent calls? The asymmetry that carry inputs are separate arguments while carry outputs are the MSB of LE registers feels like it could make that difficult to chain such that an example may be helpful. + - I'd personally recommend staging this together with https://github.com/microsoft/QuantumLibraries/issues/337 to minimize the number of breaking changes and to make sure that both APIs are as consistent with each other as possible. #337 in particular feels like it could be helpful in making the input data types consistent not only across arithmetic APIs but other places where arithmetic data is used in general-purpose APIs. + > Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. **Consensus**: From eea2d388cfffce8dc0dc6db478ece804ee1dd69c Mon Sep 17 00:00:00 2001 From: Mathias Soeken Date: Thu, 5 May 2022 22:36:45 -0700 Subject: [PATCH 3/4] Apply suggestions from code review --- Design/meetings/2022/api-design-2022-05.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Design/meetings/2022/api-design-2022-05.md b/Design/meetings/2022/api-design-2022-05.md index c2de434cba1..b819d0acc2e 100644 --- a/Design/meetings/2022/api-design-2022-05.md +++ b/Design/meetings/2022/api-design-2022-05.md @@ -1,6 +1,6 @@ # Q# API Design Discussions / May 2022 -Reviewers (in order by username): ... +Reviewers (in order by username): cgranade ## Agenda From 5ad7c39e1b0ac4211cedfb59914ac7a464ced7fa Mon Sep 17 00:00:00 2001 From: Mathias Soeken Date: Fri, 3 Jun 2022 00:19:51 -0700 Subject: [PATCH 4/4] Update api-design-2022-05.md --- Design/meetings/2022/api-design-2022-05.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Design/meetings/2022/api-design-2022-05.md b/Design/meetings/2022/api-design-2022-05.md index b819d0acc2e..3cde840925f 100644 --- a/Design/meetings/2022/api-design-2022-05.md +++ b/Design/meetings/2022/api-design-2022-05.md @@ -20,7 +20,7 @@ Reviewers (in order by username): cgranade > Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. -**Consensus**: +**Consensus**: Approved with suggestions --- @@ -32,7 +32,7 @@ Reviewers (in order by username): cgranade - **Approved.** I think this is a pretty straightforward one, and works great for replacing the old `MultiM` with something more maintainable and easier to use. > Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. -**Consensus**: +**Consensus**: Approved --- @@ -49,4 +49,4 @@ Reviewers (in order by username): cgranade > Please add a bullet point including your alias, your review result (*approve*, *reject*, *comment*), and a comment (optional when result is *approve*). Alternatively, add a line to the PR discussion incl. a reference to this issue. -**Consensus**: +**Consensus**: Re-schedule after addressing comments