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

Lint rule: When qubit reuse is not supported, prefer MResetZ over M #1473

Open
minestarks opened this issue May 3, 2024 · 6 comments
Open
Labels
enhancement New feature or request unitary-hack unitaryHACK 2024 bounty issues

Comments

@minestarks
Copy link
Member

minestarks commented May 3, 2024

We should add a rule to the linter that suggests that calls to M are replaced by MResetZ when the currently selected target's capabilities don't include QubitReuse.

use q = Qubit();
H(q);

// When Profile=Unrestricted, the qubit is reused
// When Profile=Base, this allocates a new hardware qubit
// Using MResetZ would make the behavior less ambiguous
let r = M(q);

Playground link

In this code, the linter could display a warning under M to the effect of "the currently chosen target does not support qubit use after measurement, prefer MResetZ".


CONTRIBUTORS PLEASE READ

Getting started

Welcome! Please take a look through our README to orient yourself in the repo and find instructions on how to build.

For this issue you'll want to have a working knowledge of Rust and compilers.

For documentation on how to add lints, see:
https://github.com/microsoft/qsharp/blob/main/compiler/qsc_linter/src/lib.rs

For examples of existing lints, see:

(DivisionByZero, LintLevel::Error, "attempt to divide by zero", "division by zero will fail at runtime"),
(NeedlessParens, LintLevel::Allow, "unnecessary parentheses", "remove the extra parentheses for clarity"),
(RedundantSemicolons, LintLevel::Warn, "redundant semicolons", "remove the redundant semicolons"),

(Placeholder, LintLevel::Allow, "this a placeholder", "remove after addding the first HIR lint"),

Testing

You can demonstrate that the lint works by running the playground locally (see the code example in the issue description).

Please add unit tests verifying the functionality you implemented.

Before you submit a pull request please run python ./build.py to ensure the project builds cleanly. See README for details.

Reviews

Once you have published your PR, the codeowners will automatically get tagged and we'll review shortly.

If you need clarification on an issue please tag @minestarks with your questions.

@minestarks minestarks added enhancement New feature or request needs triage unitary-hack unitaryHACK 2024 bounty issues and removed needs triage labels May 3, 2024
@Manvi-Agrawal
Copy link
Contributor

@minestarks

  • Do you think its worth adding similar lint for MeasureEachZ?
  • On a related node I was thinking if its worth adding reset variants for others namely Measure and MeasureAllZ.
  • Also, I got a bit stumped by knowing that MeasureInteger ends up resetting the states of qubits :) Maybe rename? Deprecate and rename?
    image
    image

@minestarks
Copy link
Member Author

Good questions - this originally came from @swernli so I'll let him weigh in.

@swernli
Copy link
Collaborator

swernli commented May 14, 2024

HI @Manvi-Agrawal, I definitely have thoughts on these questions.

  • Do you think its worth adding similar lint for MeasureEachZ?

Yes, definitely. MResetEachZ should be preferred over MeasureEachZ.

  • On a related node I was thinking if its worth adding reset variants for others namely Measure and MeasureAllZ.

These two are a little special... they support passing an array of qubits but will return a single measurement result, effectively doing a parity measurement that may only partially collapse the state of the qubits in the array. Given that, these are fine when they have multiple inputs but when the input is an array with a single qubit they would be better replaced. I'm not sure how easy it is to differentiate that in lints (array sizes are not known at compile time) so it might be better left for a follow up refinement.

  • Also, I got a bit stumped by knowing that MeasureInteger ends up resetting the states of qubits :) Maybe rename? Deprecate and rename?

That is mentioned in the remarks section of the docs on that API, though it could be more prominently featured in the summary. To have a non-resetting version you could use something like ResultArrayAsInt(MeasureEachZ(qubits)) so at least it's not an impossible thing, and that might be worth calling out in the doc comments on MeasureInteger.

@Manvi-Agrawal
Copy link
Contributor

@minestarks , I would like to work on this issue.

@minestarks
Copy link
Member Author

@swernli do we have any precedence for looking up the ItemId of a specific std function in the HIR by name? Thinking about how we'd identify calls to MResetZ. We have some code that looks up core library functions by name (used in HIR passes), but not std.

@swernli
Copy link
Collaborator

swernli commented May 31, 2024

Ah, that's a good point. We avoided that in the past in part because a compilation may not us the stdlib at all, so it's not guaranteed to be there. I think to get the right resolutions identified we could cheat a bit and have a bit of code that as a prerequisite compiles just the expression MResetZ with std and then checks what resolution it turns into in HIR. We could even have it be a tuple of the desired callables, something like (M, MResetZ, MeasureEachZ, MResetEachZ) which would then show up as a tuple expression with a list of resolutions in it. The real trick would be minimizing the cost of this compilation by ensuring it reuses a compiled stdlib rather then recompiling it from scratch...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request unitary-hack unitaryHACK 2024 bounty issues
Projects
None yet
Development

No branches or pull requests

3 participants