Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom measurement operations to Q# #1967

Merged
merged 41 commits into from
Oct 25, 2024
Merged

Conversation

orpuente-MS
Copy link
Contributor

@orpuente-MS orpuente-MS commented Oct 16, 2024

This PR adds the ability to declare custom measurements in Q#. Here is an example:

@Measurement()
operation __quantum__qis__mx__body(target : Qubit) : Result {
  body intrinsic;
}

The generated QIR will be,

declare void @__quantum__qis__mx__body(%Qubit*, %Result*) #1

Design Notes

  1. I choose the @Measurement() attribute over a new measurement keyword to avoid putting extra syntax into the language.
  2. A custom measurement must be a body intrinsic or a @SimulatableIntrinsic().
  3. A measurement must only take Qubits as inputs and have Results as outputs.

Notes to reviewers

This PR consists of the following steps:

  1. Adding a @Measurement() attribute to the language and piping it down to FIR.
  2. Make the generated QIR have the #1 attribute indicating an irreversible operation.
  3. Treating a measurement as a dynamic operation in RCA.
  4. Handling custom measurements correctly during partial evaluation: moving the output Results to result registers in the input.
  5. Handling undesired semantics as errors using an HIR pass.
  6. Adding the @Measurement() attribute to the completions list.
  7. Unit tests in codegen and in the HIR pass.

Next steps

After this PR is merged we can add the @Measurement() attribute to the measurement operations in the standard library and remove the hard coded logic we have for them in the compiler.

@orpuente-MS orpuente-MS changed the title Oscarpuente/custom measurements Add custom measurement operations to Q# Oct 16, 2024
@minestarks
Copy link
Member

@orpuente-MS Do we really need an explicit @Measurement attribute if we simply define a "measurement intrinsic" as an "intrinsic that returns a Result"?

@swernli
Copy link
Collaborator

swernli commented Oct 17, 2024

@orpuente-MS Do we really need an explicit @Measurement attribute if we simply define a "measurement intrinsic" as an "intrinsic that returns a Result"?

I was having the same thought, and I think it's worth exploring. With the attribute, you must include the attribute for something to get treated as a CallableKind::Measurement, and then the operation must adhere to these properties:

  • Must only take arguments of type Qubit
  • Must only return type Result or tuple of Result
  • Must be body intrinsic or @SimulatableIntrinsic

If instead these restrictions became checks during lowering from AST to HIR, then any operation that meets these requirements could be lowered into CallableKind::Measurement. Then the user wouldn't have to remember to add the attribute AND only valid callables would have the corresponding kind. It's possible there is a corner case this doesn't cover, but it seems initially like this could work well.

@orpuente-MS
Copy link
Contributor Author

orpuente-MS commented Oct 17, 2024

My one concern with this is that the compiler won't help the user with friendly errors if the user doesn't use the right combination of inputs & outputs when trying to write a measurement. For example, if the user writes this,

operation __quantum__qis__mx__body(target : Qubit, invalid_arg: Int) : (Int, String) {
  body intrinsic;
}

and they intended to write a measurement, we have no way to guide them towards the right path by telling them:

Hey if you intended to write a measurement you should satisfy these conditions...

because this could perfectly be a non-measurement intrinsic, so we can't issue errors in this case

@swernli
Copy link
Collaborator

swernli commented Oct 17, 2024

That's a good point, and I think we can focus in on that problem statement: if someone writes something that is valid Q# in general but we know it will fail QIR codegen because it doesn't match the set of intrinsic qualities we support, how do we tell them? I have some thoughts on what that feedback could look like, so perhaps you, @cesarzc and I could brainstorm a little.

Overall, I do think a lot of the infrastructure in this PR could be used to get toward this goal, even if the specific detail of an @Measurement attribute itself gets dropped.

@orpuente-MS
Copy link
Contributor Author

orpuente-MS commented Oct 17, 2024

In general I don't like the idea of having a special kind of operation (i.e. measurements) hidden behind a specific set of inputs & outputs. I think we should be explicit about measurement declarations. Initially I wanted to add a measurement keyword so that we could have:

measurement Mx(target: Qubit) : Result {
  body intrinsic;
}

I like this because it feels like measurements are actually part of the language, which is nice because they are treated differently in literature and in our codegen.

The reason I decided not to go this route is because it adds a new keyword to the language, and that could break existing user code.

@orpuente-MS
Copy link
Contributor Author

orpuente-MS commented Oct 17, 2024

I forgot to mention that @Measurements() can also return Unit. The reason is that in this way we can express Reset or collapsing operations in general as measurements, which seems appropriate because they are irreversible, so they should have the #1 attribute in QIR?

Note that this is not a special case because you can think of it as "a measurement must return 0 or more Results".

@Measurement()
operation Projection(target: Qubit) : Unit {
  body intrinsic;
}

In this case it is critical that we have a @Measurement attribute, because operations like H have the same signature.

operation H(target: Qubit) : Unit {
  body intrinsic;
}

@swernli
Copy link
Collaborator

swernli commented Oct 17, 2024

Yes, Reset is an interesting special case, that I think may need to remain a special case for a while. It should definitely have the "irreversible" QIR attribute and today that is missing. But it also doesn't produce a measurement result anywhere, so it doesn't get the same treatment as others (having its signature changed, getting deferred, etc). Also, when you consider how we do the deferral, there is a difference between measurement and reset (or to put it another way, non-destructive vs destructive measurement) in terms of whether the new, replacement qubit needs to have the corresponding state as the original.

I think you've identified another issue here: how do we (or the user) express that an operation is destructive to the state? As you point out, maybe it's that scenario that requires the attribute because it can't be differentiated based on signature alone.

@orpuente-MS
Copy link
Contributor Author

Yes, Reset is an interesting special case, that I think may need to remain a special case for a while. It should definitely have the "irreversible" QIR attribute and today that is missing. But it also doesn't produce a measurement result anywhere, so it doesn't get the same treatment as others (having its signature changed, getting deferred, etc). Also, when you consider how we do the deferral, there is a difference between measurement and reset (or to put it another way, non-destructive vs destructive measurement) in terms of whether the new, replacement qubit needs to have the corresponding state as the original.

Ohh, I didn't know that Reset was that different, thank you for clarifying. I edited my last message to use Projection instead of Reset to make my point better.

Copy link
Collaborator

@swernli swernli left a comment

Choose a reason for hiding this comment

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

One minor fix for help message left, otherwise it looks good!

compiler/qsc_frontend/src/lower.rs Outdated Show resolved Hide resolved
@orpuente-MS orpuente-MS enabled auto-merge October 25, 2024 19:00
@orpuente-MS orpuente-MS added this pull request to the merge queue Oct 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 25, 2024
@swernli swernli added this pull request to the merge queue Oct 25, 2024
Merged via the queue into main with commit a37c324 Oct 25, 2024
18 checks passed
@swernli swernli deleted the oscarpuente/custom-measurements branch October 25, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants