Add act_on protocol #3019
Add act_on protocol #3019
Conversation
Strilanc
commented
May 18, 2020
Strilanc
commented
- Add act_on_protocol.py
- Add act_on to MeasurementGate and ResetChannel
- Add ActOnStateVectorArgs to represent state vector simulator state
- Move logic out of sparse_simulator.py and into ActOnStateVectorArgs._fallback_act_on
- Fix ApplyUnitaryArgs.subspace_index not forwarding shape information
- Add allow_decompose to several protocol methods
- Fix CliffordSimulator accepting measurements that may not be stabilizers
- Fix _verify_unique_measurement_keys not using measurement_keys
- Add test that terminal measurement simulations can have random results
- with_gate now returns self if new_gate is gate
- GateOperation now delegates protocols directly to its gate when possible
- Add act_on_protocol.py - Add _act_on_ to MeasurementGate and ResetChannel - Add ActOnStateVectorArgs to represent state vector simulator state - Move logic out of sparse_simulator.py and into ActOnStateVectorArgs._fallback_act_on - Fix ApplyUnitaryArgs.subspace_index not forwarding shape information - Add allow_decompose to several protocol methods - Fix CliffordSimulator accepting measurements that may not be stabilizers - Fix _verify_unique_measurement_keys not using measurement_key**s** - Add test that terminal measurement simulations can have random results - with_gate now returns self if new_gate is gate - GateOperation now delegates protocols directly to its gate when possible
| @@ -114,33 +116,57 @@ def _decompose_(self) -> 'cirq.OP_TREE': | |||
| NotImplemented) | |||
|
|
|||
| def _pauli_expansion_(self) -> value.LinearDict[str]: | |||
| return protocols.pauli_expansion(self.gate) | |||
There was a problem hiding this comment.
Can you explain the advantage of delegating in these methods? I don't see why this is better and adds a lot of boilerplate getattr code.
There was a problem hiding this comment.
Sure. The triggering event for me is that the allow_decompose parameter was not being respected in several places when GateOperation was in play. For example, has_unitary(gate_op, allow_decompose=True) would call GateOperation._has_unitary_, which would call has_unitary(gate) with no allow_decompose=True, resulting in the decomposition being computed.
Basically, if we bounce a protocol method back into itself then we can get an amplification effect where more work than necessary is done because fallback logic is run once per bounce-back-level. By delegating directly into the implementation of the sub gate, fallback logic is only run once. This makes things more efficient, and honestly a bit more stable or at least predictable.
There was a problem hiding this comment.
Well about decompose :). We should probably make this the norm then throughout.
cirq/sim/act_on_state_vector_args.py
Outdated
| expected to perform inplace edits of this object. | ||
| available_buffer: A workspace with the same shape and dtype as | ||
| `target_tensor`. The result of an operation can be put into this | ||
| buffer, instead of directly editing `target_tensor`, if |
There was a problem hiding this comment.
I might reword this because it's a bit hard to parse. I think what you are saying is that if the result is put into the avaliable buffer instead of acting on inline, then swap_target_tensor_for should be called to put this in target_tensor?
| def __init__(self, target_tensor: np.ndarray, available_buffer: np.ndarray, | ||
| axes: Iterable[int], prng: np.random.RandomState, | ||
| log_of_measurement_results: Dict[str, Any]): | ||
| """ |
There was a problem hiding this comment.
It's probably useful to describe the different use cases, inline, using a buffer, and creating a new vector?
cirq/sim/act_on_state_vector_args.py
Outdated
| bit of the integer is the desired bit for the first axis, and | ||
| so forth in decreasing order. Can't be specified at the same | ||
| time as `little_endian_bits_int`. | ||
| value_tuple: The desired value of the qids at the targeted `axes`, |
There was a problem hiding this comment.
this parameter doesn't exist in this method
Strilanc
added
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
the
automerge
CirqBot
removed
the
front_of_queue_automerge
- Add act_on_protocol.py - Add _act_on_ to MeasurementGate and ResetChannel - Add ActOnStateVectorArgs to represent state vector simulator state - Move logic out of sparse_simulator.py and into ActOnStateVectorArgs._fallback_act_on - Fix ApplyUnitaryArgs.subspace_index not forwarding shape information - Add allow_decompose to several protocol methods - Fix CliffordSimulator accepting measurements that may not be stabilizers - Fix _verify_unique_measurement_keys not using measurement_key**s** - Add test that terminal measurement simulations can have random results - with_gate now returns self if new_gate is gate - GateOperation now delegates protocols directly to its gate when possible
Fixes #3825 Sparse and Clifford simulators both have a proper ActOnXXXStateArgs where simulator state is maintained and acted upon by various operations. This was first added in #3019 with the act_on protocol. DensityMatrix and MPS simulators were never updated with the new protocol, leaving the codebase inconsistent and the migration unfinished. This PR finishes the task for DensityMatrix. A separate PR for MPS simulator hopefully will follow. Aditionally, this PR introduces an `ActOnArgs` base class to eliminate the code duplication and type checking that was required in `MeasurementGate` (it had grown to four `if isinstance` blocks, each with duplicate bitshift/logging code), and replace it all with a single function call back into the `args` object.
