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

Classical sympy conditions with qudits #4778

Closed
wants to merge 47 commits into from
Closed

Conversation

daxfohl
Copy link
Contributor

@daxfohl daxfohl commented Dec 23, 2021

Part 14 of https://tinyurl.com/cirq-feedforward, the final PR of the series(!)

Adds a measured_qubits field to ActOnArgs, that records which qubits were used in each measurement. Then uses the dimensions of these qubits when calculating the integer value of each measurement when resolving sympy expressions.

Closes #3232
Closes #887

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

@CirqBot CirqBot added the size: M 50< lines changed <250 label Dec 23, 2021
@daxfohl daxfohl marked this pull request as ready for review December 23, 2021 22:33
@daxfohl daxfohl requested review from cduck, vtomole, wcourtney and a team as code owners December 23, 2021 22:33
@daxfohl daxfohl requested a review from dabacon December 23, 2021 22:33
@@ -47,6 +47,7 @@ def __init__(
prng: np.random.RandomState = None,
qubits: Sequence['cirq.Qid'] = None,
log_of_measurement_results: Dict[str, List[int]] = None,
measured_qubits: Dict[str, Tuple['cirq.Qid', ...]] = None,
Copy link
Contributor Author

@daxfohl daxfohl Dec 24, 2021

Choose a reason for hiding this comment

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

@95-martin-orion I did this the least-invasive way, of adding this measured_qubits member and propagating it everywhere it needed to be. This is a little smelly because it and log_of_measurement_results go hand in hand and are initialized, copied, etc. in pairs. I think the better option though is to introduce a new classical_state: ClassicalState that contains both of these dictionaries, plus whatever we add later, and then deprecate log_of_measurement_results.. I know it breaks the "rule of three" to do so, but I'm pretty sure we'll have a third thing to put into it soon.

It may be a big deprecation, but it may be worth it to do now rather than having to clean up the mess after things like repeated measurements, keeping track of qubits, etc have all become dependent on it.

Additionally I noticed a minor bug here is that I don't keep track of the qubits in keyed channels. If we have a ClassicalState class then we can make the interface such that writing to the dictionaries requires both a value and a qubit list, so that such an omission is impossible. Of course, that will make for a much bigger review. WDYT?

@daxfohl daxfohl closed this Dec 24, 2021
CirqBot pushed a commit that referenced this pull request 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 daxfohl deleted the qudits branch February 7, 2022 20:32
rht pushed a commit to rht/Cirq that referenced this pull request 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 pull request 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
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Classical Control: Feedforward Feature request: classical registers
2 participants