Skip to content

RFC: refactoring measurement #158

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

Open
Roger-luo opened this issue Apr 15, 2025 · 16 comments
Open

RFC: refactoring measurement #158

Roger-luo opened this issue Apr 15, 2025 · 16 comments
Assignees
Labels
breaking breaking changes or proposed changes that would break existing APIs enhancement New feature or request rfc Request for Comments squin squin related issues

Comments

@Roger-luo
Copy link
Member

Roger-luo commented Apr 15, 2025

The Measure statement is not very compatible with stim or QASM2

from stim there are

  • PPMeasure(p: float, pauli_string) -> bool: measure on a Pauli string as basis by a given probability
    • measure XXYY on 4 qubits resulting a boolean value
    • p is the probablity of -> bool being flipped
    • MZZ, etc.
  • Measure(basis, position, p: float) -> bool

from QASM2 there are

  • Measure(qreg, creg)

We introduce the following statement to model all the above

# in wire
class Measure:
    basis: ir.SSAValue = info.argument(OpType)
    wires: tuple[ir.SSAValue, ...] = info.argument(Wire)
    prob: ir.SSAValue = info.argument(Union[Float, NoneType])
# in qubit
class Measure:
    basis: ir.SSAValue = info.argument(OpType)
    qubits: ir.SSAValue = info.argument(IListType[Qubit, ...])
    prob: ir.SSAValue = info.argument(Union[Float, NoneType])
parallel {
    %1 = Measure(basis, (%wire_1, %wire_2), p=None) -> bool
    %2 = Measure(basis, (%wire_3, %wire_4), p=None) -> bool
}

cc: @kaihsin @johnzl-777

@Roger-luo Roger-luo added squin squin related issues enhancement New feature or request breaking breaking changes or proposed changes that would break existing APIs labels Apr 15, 2025
@cduck
Copy link

cduck commented Apr 15, 2025

Is prop the probability of a measurement error? I'm not sure it should be included in a general Measure instruction. If you need to represent errors for simulation, simply do Depolarizing(wires, px=prob, py=0, pz=0); Measure('Z', wires).

@kaihsin
Copy link
Contributor

kaihsin commented Apr 15, 2025

How about PPMeasure? This issue also try to unify them together

@Roger-luo
Copy link
Member Author

Is prop the probability of a measurement error?

yes, the prob is the error of measurement output being flipped.

I'm not sure it should be included in a general Measure instruction. If you need to represent errors for simulation, simply do Depolarizing(wires, px=prob, py=0, pz=0); Measure('Z', wires).

I think this makes sense to me, this is due to a noise channel. I'm actually wondering what are the corresponding noise operator for a PPMeasure here?

@cduck
Copy link

cduck commented Apr 15, 2025

Neutral atoms don't directly measure Pauli products so there is no direct noise model. You have to decompose it into a circuit with an additional ancilla. Measuring the Pauli string X1Y2Z3:

Image

The noise depends on how you execute this circuit so I don't think including a "probability" argument at this IR level makes any sense.

@kaihsin
Copy link
Contributor

kaihsin commented Apr 15, 2025 via email

@cduck
Copy link

cduck commented Apr 15, 2025

I don't think we have much use for a squin.PPMeasure instruction so is it OK for squin to not 1-to-1 map to stim? And noise can be a separate noise_op dialect that is compatible with squin and can be added by a noise estimation pass.

@kaihsin
Copy link
Contributor

kaihsin commented Apr 15, 2025 via email

@weinbe58
Copy link
Member

I agree that we should decouple the noise from the measurement. Even semantically, it seems redundant.

@zphy
Copy link

zphy commented Apr 21, 2025

Agreed with Casey's comment above - physically, noise should be separate, and we don't have native multi-body measurements. In QEC research, the main place we make use of the MPP command is when defining a logical observable, consisting of the product of physical Pauli operators on a bunch of sites, but this can be done in post-processing as well so is not a required functionality.

@kaihsin
Copy link
Contributor

kaihsin commented Apr 21, 2025

Sounds like a 4 yeses! Let's do it.

@johnzl-777
Copy link
Contributor

Okay so just to make concrete what I understand here and what should be implemented:

  • There's no need for something like MPP at the squin level
  • Noise shouldn't be "tied in" with measurement statements at all and can be inserted via another pass + represented with a different noise_op dialect

Thus, with some minor modification the new Measure statements (piggybacking off of @Roger-luo 's initial proposal) are now just:

# in wire
class Measure:
    basis: ir.SSAValue = info.argument(OpType)
    wires: tuple[ir.SSAValue, ...] = info.argument(Wire)
# in qubit
class Measure:
    basis: ir.SSAValue = info.argument(OpType)
    qubits: ir.SSAValue = info.argument(IListType[Qubit, ...])

Now one thing I would want to make clear here is in the following:

parallel {
    %1 = Measure(basis, (%wire_1, %wire_2), p=None) -> bool
    %2 = Measure(basis, (%wire_3, %wire_4), p=None) -> bool
}

I assume the bool here would be a proper ir.ResultValue? Although this would still run into the problems discussed in #148

@Roger-luo
Copy link
Member Author

you missed the return value of measure in the its definition. I think the return value should be list of bool, not bool. For parallel

%3, %4 = parallel {
  %1 = measure(%basis, %wire_1, %wire_2) -> IList[bool]
  %2 = measure(%basis, %wire_1, %wire_2) -> IList[bool]
  yield %1, %2
}

@weinbe58
Copy link
Member

I thought we were going to remove parallel? If you have measurements that can happen in parallel just merge the arguments?

@Roger-luo
Copy link
Member Author

I thought we were going to remove parallel? If you have measurements that can happen in parallel just merge the arguments?

that's right. We are adding a broadcast/repeat in the operator language.

@johnzl-777
Copy link
Contributor

From a meeting with myself, @david-pl , @Roger-luo , and @weinbe58 the syntax for Measure should be the following.

@overload
def measure(op, q: Qubit) -> bool:
    pass

measure(op, q[0])

@overload
def measure(op, args: IList[Qubit]) -> list[bool]:
    pass

@weinbe58 has asked me to prioritize working on #179 and the (immediately) necessary sub-issues so I'll hand the reigns over to him on this issue 👍

@johnzl-777 johnzl-777 assigned weinbe58 and unassigned johnzl-777 Apr 25, 2025
@johnzl-777 johnzl-777 added the rfc Request for Comments label Apr 25, 2025
@johnzl-777
Copy link
Contributor

Also want to point out that as of now, Measure as well as MeasureAndReset in the wire dialect have integer types for their results. Updating #180 to align with developments here.

weinbe58 added a commit that referenced this issue Apr 29, 2025
This PR addresses #158
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking breaking changes or proposed changes that would break existing APIs enhancement New feature or request rfc Request for Comments squin squin related issues
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

6 participants