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

Keeping track of qubits you've measured #4449

Open
mpharrigan opened this issue Aug 20, 2021 · 10 comments
Open

Keeping track of qubits you've measured #4449

mpharrigan opened this issue Aug 20, 2021 · 10 comments
Labels
area/measurements area/qubits kind/design-issue A conversation around design triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@mpharrigan
Copy link
Collaborator

@JiahaoYao wanted to take a circuit where we were measuring 1 of the qubits to a case where we're measuring all of the qubits. Sounds like there should be an obvious one way to do this? I wrote the following in response, reproduced here so that others may 1) use this advice or 2) think about why there's no idiomatic way of doing this. cc #4144

Use cirq.measure(*qubits) instead of cirq.measure(q) for q in qubits (where qubits is a list). Now the results will have a 2d numpy array where the second axis is the qubit index in the order they were listed in qubits

You still need to save a mapping between the qubit and the index. One way of doing this is a dictionary {q: i for i, q in enumerate(qubits)}. Since you can always do this on the fly, I prefer just saving the qubits ordered list alongside my circuit so I can make this "mapping" on the fly

Some people use sorted(circuit.all_qubits()) as an implicit qubit ordering everywhere, but I worry this can introduce subtle bugs if you forget to sort your qubits sometimes or e.g. qubits have different sort order after you place your circuit onto a device. I prefer having an explicit qubits list.

Some people like using cirq.measure(q, key=str(q)) for q in qubits and parsing the stringy qubit key. This feels dirty and precludes lots of numpy tricks, e.g. zz_parity = np.sum(bitstrings[:, [i0, i1]], axis=1) % 2

@mpharrigan mpharrigan added the kind/design-issue A conversation around design label Aug 20, 2021
@maffoo
Copy link
Contributor

maffoo commented Aug 20, 2021

This is the kind of stuff we deal with all the time at the hardware level, keeping track of what data for each key corresponds to what qubits, so I have some thoughts in this area.

Note that we have a find_measurements helper function in cirq_google to extract information about all measurements in a circuit: https://github.com/quantumlib/Cirq/blob/master/cirq-google/cirq_google/api/v2/results.py#L58. It returns a list of MeasureInfo dataclasses, one for each measurement, that include the measured qubits. It's a bit specialized for google devices, e.g. using cirq.GridQubit instead of cirq.Qid, but in general it's helpful to extract this kind of info from a circuit to help interpret results.

(Aside: One problem I always run into with this kind of analysis is I'd like to be able to compute some property of a circuit, e.g. find info about measurements, and then cache the result on the circuit so I don't accidentally redo the computation. This is harder than it ought to be because circuits are mutable. Having a generic way to cache such analyses and then invalidate them if the circuit is mutated would be useful.)

We could have cirq.Result include some of this information, such as storing the qubits along with the data for each measure key, though if we remove the uniqueness constraint keys as proposed in #4274 it could be that each "instance" of a particular key in a circuit measures different qubits, which would complicate the tracking of what data corresponds to what qubits.

@maffoo maffoo closed this as completed Aug 20, 2021
@maffoo maffoo reopened this Aug 20, 2021
@maffoo
Copy link
Contributor

maffoo commented Aug 20, 2021

Not sure I really addressed your question, though. I agree with cirq.measure(*qubits) and keeping track of the qubit order, so maybe the one thing we might consider is including some kind of "measurement descriptor" in cirq.Result so if you save results you don't have to separately keep track of the qubit order.

@mpharrigan
Copy link
Collaborator Author

I should also add cirq.measure(*qubits) keeps all your measure operation in one moment, which is really important.

@viathor viathor added area/measurements area/qubits triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Aug 25, 2021
@dabacon dabacon self-assigned this Nov 10, 2021
@dstrain115
Copy link
Collaborator

Cirq cync requests an update from @dabacon whether this should be accepted.

@dabacon
Copy link
Collaborator

dabacon commented Dec 1, 2021

I think storing information about the qubits that are measured on Result seems like a good idea. I think when we've talked about this in the past, it was always pointed out that you could keep track of this yourself, but this seems like a lot of ugly code to have to write.

@daxfohl
Copy link
Contributor

daxfohl commented Dec 1, 2021

The simulators don't currently track what measurements correspond to what qubits. But I was planning to add this in the near future anyway so that we can support qudits in integer expressions in classical control: if we have cirq.measure(some_qudits, key='mkey'), cirq.X(q0).if('mkey > 20'), then we need to retain at least the shape of the qudits we measured so that we can calculate the int from the dits.

Once that's done we should be able to pass that info into Result easily enough.

@tanujkhattar tanujkhattar added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jan 26, 2022
CirqBot pushed a commit that referenced this issue Feb 7, 2022
Adds a `ClassicalDataStore` class so we can keep track of which qubits are associated to which measurements.

Closes #3232. Initially this was created as part 14 (of 14) of https://tinyurl.com/cirq-feedforward to enable qudits in classical conditions, by storing and using dimensions of the measured qubits when calculating the integer value of each measurement when resolving sympy expressions. However it may have broader applicability.

This approach also sets us up to more easily add different types of measurements (#3233, #4274). It will also ease the path to #3002 and #4449., as we can eventually pass this into `Result` rather than the raw `log_of_measurement_results` dictionary. (The return type of `_run` will have to be changed to `Sequence[C;assicalDataStoreReader]`.

Related: #887, #3231 (open question @95-martin-orion whether this closes those or not)

This PR contains a `ClassicalDataStoreReader` and `ClassicalDataStoreBase` parent "interface" for the `ClassicalDataStore` class as well. This will allow us to swap in different representations that may have different performance characteristics. See #3808 for an example use case. This could be done by adding an optional `ClassicalDataStore` factory method argument to the `SimulatorBase` initializer, or separately to sampler classes.

(Note this is an alternative to #4778 for supporting qudits in sympy classical control expressions, as discussed here: https://github.com/quantumlib/Cirq/pull/4778/files#r774816995. The other PR was simpler and less invasive, but a bit hacky. I felt even though bigger, this seemed like the better approach and especially fits better with our future direction, and closed the other one).

**Breaking Changes**:
1. The abstract method `SimulatorBase._create_partial_act_on_args` argument `log_of_measurement_results: Dict` has been changed to `classical_data: ClassicalData`. Any third-party simulators that inherit `SimulatorBase` will need to update their implementation accordingly.
2. The abstract base class `ActOnArgs.__init__` argument `log_of_measurement_results: Dict` is now copied before use. For users that depend on the pass-by-reference semantics (this should be rare), they can use the new `classical_data: ClassicalData` argument instead, which is pass-by-reference.
@daxfohl
Copy link
Contributor

daxfohl commented Feb 18, 2022

Unassigning myself but this should be fairly doable now. All the data is stored in the simulation TrialResult class, and just needs passed into the Result class. It might require breaking change to SimulatesSamples._run return type. That currently just returns raw data. If we can have it return the collection of TrialResults instead, then those could be passed into ResultDict to satisfy this requirement. Note qsim would be affected as well.

@daxfohl daxfohl removed their assignment Feb 18, 2022
@95-martin-orion
Copy link
Collaborator

All the data is stored in the simulation TrialResult class, and just needs passed into the Result class.

The data is stored in the optional final_step_result field, which is only available to SimulatesIntermediateStates simulators. Since there's no guarantee that a SimulatesSamples simulators implements intermediate states (qsim doesn't, for one) we can't use it here - or at least not in a generic way that covers all simulators.

@daxfohl
Copy link
Contributor

daxfohl commented Feb 23, 2022

Could we override SimulatesSamples.run_sweep_iter in SimulatorBase? Anything that inherits from SimulatorBase should have this final_step_result field hydrated. Thus SimulatorBase.run_sweep_iter should be able to return Results that have the qubit information (either a new Result subclass, or add the functionality to ResultDict). Then we can revert SimulatesSamples.run_sweep_iter back to expecting a 2D array, which would fix the qsim (and any others) integration.

@95-martin-orion
Copy link
Collaborator

I don't think modifying SimulatorBase is sufficient to resolve this problem, as it's not extensible to external simulators. RE use of 2D arrays, I've opened #5014 with a fairly lightweight fix.

@dabacon dabacon removed their assignment Jan 11, 2023
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
…mlib#4781)

Adds a `ClassicalDataStore` class so we can keep track of which qubits are associated to which measurements.

Closes quantumlib#3232. Initially this was created as part 14 (of 14) of https://tinyurl.com/cirq-feedforward to enable qudits in classical conditions, by storing and using dimensions of the measured qubits when calculating the integer value of each measurement when resolving sympy expressions. However it may have broader applicability.

This approach also sets us up to more easily add different types of measurements (quantumlib#3233, quantumlib#4274). It will also ease the path to quantumlib#3002 and quantumlib#4449., as we can eventually pass this into `Result` rather than the raw `log_of_measurement_results` dictionary. (The return type of `_run` will have to be changed to `Sequence[C;assicalDataStoreReader]`.

Related: quantumlib#887, quantumlib#3231 (open question @95-martin-orion whether this closes those or not)

This PR contains a `ClassicalDataStoreReader` and `ClassicalDataStoreBase` parent "interface" for the `ClassicalDataStore` class as well. This will allow us to swap in different representations that may have different performance characteristics. See quantumlib#3808 for an example use case. This could be done by adding an optional `ClassicalDataStore` factory method argument to the `SimulatorBase` initializer, or separately to sampler classes.

(Note this is an alternative to quantumlib#4778 for supporting qudits in sympy classical control expressions, as discussed here: https://github.com/quantumlib/Cirq/pull/4778/files#r774816995. The other PR was simpler and less invasive, but a bit hacky. I felt even though bigger, this seemed like the better approach and especially fits better with our future direction, and closed the other one).

**Breaking Changes**:
1. The abstract method `SimulatorBase._create_partial_act_on_args` argument `log_of_measurement_results: Dict` has been changed to `classical_data: ClassicalData`. Any third-party simulators that inherit `SimulatorBase` will need to update their implementation accordingly.
2. The abstract base class `ActOnArgs.__init__` argument `log_of_measurement_results: Dict` is now copied before use. For users that depend on the pass-by-reference semantics (this should be rare), they can use the new `classical_data: ClassicalData` argument instead, which is pass-by-reference.
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
…mlib#4781)

Adds a `ClassicalDataStore` class so we can keep track of which qubits are associated to which measurements.

Closes quantumlib#3232. Initially this was created as part 14 (of 14) of https://tinyurl.com/cirq-feedforward to enable qudits in classical conditions, by storing and using dimensions of the measured qubits when calculating the integer value of each measurement when resolving sympy expressions. However it may have broader applicability.

This approach also sets us up to more easily add different types of measurements (quantumlib#3233, quantumlib#4274). It will also ease the path to quantumlib#3002 and quantumlib#4449., as we can eventually pass this into `Result` rather than the raw `log_of_measurement_results` dictionary. (The return type of `_run` will have to be changed to `Sequence[C;assicalDataStoreReader]`.

Related: quantumlib#887, quantumlib#3231 (open question @95-martin-orion whether this closes those or not)

This PR contains a `ClassicalDataStoreReader` and `ClassicalDataStoreBase` parent "interface" for the `ClassicalDataStore` class as well. This will allow us to swap in different representations that may have different performance characteristics. See quantumlib#3808 for an example use case. This could be done by adding an optional `ClassicalDataStore` factory method argument to the `SimulatorBase` initializer, or separately to sampler classes.

(Note this is an alternative to quantumlib#4778 for supporting qudits in sympy classical control expressions, as discussed here: https://github.com/quantumlib/Cirq/pull/4778/files#r774816995. The other PR was simpler and less invasive, but a bit hacky. I felt even though bigger, this seemed like the better approach and especially fits better with our future direction, and closed the other one).

**Breaking Changes**:
1. The abstract method `SimulatorBase._create_partial_act_on_args` argument `log_of_measurement_results: Dict` has been changed to `classical_data: ClassicalData`. Any third-party simulators that inherit `SimulatorBase` will need to update their implementation accordingly.
2. The abstract base class `ActOnArgs.__init__` argument `log_of_measurement_results: Dict` is now copied before use. For users that depend on the pass-by-reference semantics (this should be rare), they can use the new `classical_data: ClassicalData` argument instead, which is pass-by-reference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/measurements area/qubits kind/design-issue A conversation around design triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

No branches or pull requests

9 participants