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

Add new MeasureIfAllZeros operation. #204

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

cgranade
Copy link
Contributor

No description provided.

@cgranade
Copy link
Contributor Author

@tcNickolas: I've gone on and added this operation as per your feedback, please let me know if it addresses your usecase before I merge it in. Thanks!

Standard/src/Measurement/Registers.qs Outdated Show resolved Hide resolved
/// $\ket{00 \cdots 0}$ state.
///
/// # Remarks
/// This operation does not reset its qubits, but projects them to a
Copy link
Member

Choose a reason for hiding this comment

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

This behavior differs from that of MeasureInteger - any specific reason why? It seems more neat to be able to write "return MeasureIfAllZeros(register)" than to measure into a variable, then call ResetAll, then return that variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intent was to match the semantics of other operations in the Microsoft.Quantum.Measurement namespace; the operations in Microsoft.Quantum.Arithmetic don't follow the same convention (they should be reconciled, but that's a slightly separate discussion).

For the usecase of returning after, I could imagine something like ResetAfter<'T>(op : (Qubit[] => 'T), target : Qubit[]) : 'T { let returnValue = op(target); ResetAll(target); return returnValue; } such that you could do return ResetAfter(MeasureIfAllZeros(register));. That would be less efficient in terms of the number of measurements than return All(ForEach(MResetZ, target)), though.

Copy link
Member

Choose a reason for hiding this comment

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

In the last code snippet, did you mean something like All(IsResultZero, ForEach(MResetZ, target))?

From the end user point of view, it might be still more convenient to stick with MeasureInteger than to use MeasureIfAllZeros with variable assignment or to chain together several operations from different namespaces...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thank you; that's the danger of writing code quickly on the way to a meeting, I suppose. Anyway, I think there's two distinct issues that are getting a bit conflated here:

  • Existing measurement operations expose different reset semantics based on whether they are in Microsoft.Quantum.Measurement or Microsoft.Quantum.Arithmetic.
  • Both kinds of reset semantics have independent usecases, and should be made available to uesrs.

Standard/src/Measurement/Registers.qs Outdated Show resolved Hide resolved
@cgranade cgranade added the Status-NeedsApiReview This PR requires an API review before merging in. label Feb 5, 2020
@microsoft-github-updates microsoft-github-updates bot changed the base branch from master to main September 9, 2020 07:52
@msoeken
Copy link
Member

msoeken commented Apr 22, 2022

@cgranade, could you please submit an issue for an API review of the proposed operation. We can then schedule it for API review in order to complete this PR. If the PR is no longer of interest, we can close it alternatively. Thanks

@cgranade
Copy link
Contributor Author

@cgranade, could you please submit an issue for an API review of the proposed operation. We can then schedule it for API review in order to complete this PR. If the PR is no longer of interest, we can close it alternatively. Thanks

Sounds good, can do that. I think this also originally came your suggestion, @tcNickolas, if you'd prefer to file the issue?

@tcNickolas
Copy link
Member

@cgranade I'll admit I have no recollection of suggesting this... My best guess it had something to do with checking whether the oracles modify their input registers, this could've been something I did two years ago?

I think it will be better if you do the proposal - from the comments it looks like you had thoughts on offering different variants of this operation rather than just one? And then I chime in on the API review step

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status-NeedsApiReview This PR requires an API review before merging in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants