Update Density Matrix and State Vector Simulators to work when an operation allocates new qubits as part of its decomposition #6108
Conversation
…ng ancillas in state vector and density matrix simulators
There was a problem hiding this comment.
I'll wait for you to resolve Dax's comments and make the tests pass and then I'll take a look. In the meantime, here is a relevant discussion where I list out a bunch of different cases for "Gates which can allocate new qubits" that should be added as part of tests. #6112 (review)
If that PR is merged before this one, you can reuse the gates for testing the changes that you make to simulators by importing them from unitary_protocol_test.py.
In the meantime, please let me know if you are stuck or have any questions.
There was a problem hiding this comment.
I'll wait for you to resolve Dax's comments and make the tests pass and then I'll take a look. In the meantime, here is a relevant discussion where I list out a bunch of different cases for "Gates which can allocate new qubits" that should be added as part of tests. #6112 (review)
If that PR is merged before this one, you can reuse the gates for testing the changes that you make to simulators by importing them from unitary_protocol_test.py.
In the meantime, please let me know if you are stuck or have any questions.
…quent push will add more test cases per Tanuj's comment
| for test_simulator in ['cirq.final_state_vector', 'cirq.final_density_matrix']: | ||
| test_sim = eval(test_simulator)(test_circuit) | ||
| control_sim = eval(test_simulator)(control_circuit) | ||
| cirq.testing.assert_allclose_up_to_global_phase(test_sim, control_sim, atol=1e-6) |
There was a problem hiding this comment.
@daxfohl Do you know if we mess up the global phase as part of simulations ? The factor / kron methods are messing up global phase since if I replace PhaseUsingCleanAncilla(theta=theta, ancilla_bitsize=num_ancilla).on(q) with cirq.MatrixGate(cirq.unitary(PhaseUsingCleanAncilla(theta=theta, ancilla_bitsize=num_ancilla))).on(q); the tests pass using assert np.allclose(test_sim, control_sim) instead of
cirq.testing.assert_allclose_up_to_global_phase(test_sim, control_sim, atol=1e-6).
Ideally, the resulting state vectors and density matrices should be identical and not equal upto global phase, but the factor / kron (or some other interaction of these methods in this PR) seems to mess up the global phase here.
There was a problem hiding this comment.
Okay, so what is happening is as follows:
state_to_factor = (0.63+0.33j)|00⟩ + (-0.2+0.67j)|10⟩
# StateVectorSimulationState factors the above state into e, r s.t.
e= (0.88+0.47j)|0⟩
r= 0.71|0⟩ + (0.14+0.69j)|1⟩
# Now, we just ignore `e` and return `r`; which has the wrong global phase.
@daxfohl Do we not encounter this problem when factoring out measured / reset qubits when we construct SimulationProductState ? I guess we can just multiply the extra phase to correct it, but curious if this is known pattern elsewhere as well? Seems very relevant.
There was a problem hiding this comment.
Well, I guess we never really "discard" qubits when constructing a SimulationProductState from unentangled states, and therefore this issue doesn't arise?
There was a problem hiding this comment.
Added a fix in tanujkhattar@1db8ac5
There was a problem hiding this comment.
I thought I fixed this in #5847. What is the difference in this scenario?
There was a problem hiding this comment.
I see. Probably the linalg functions should take an extra bool parameter to force all phase into the reminder, and verify that the extracted axes are left in a classical basis state. All use cases of those functions, that's what they're ultimately trying to do.
…ration allocates new qubits as part of its decomposition (quantumlib#6108) * WIP add factoring and kron methods to sim state for adding and removing ancillas in state vector and density matrix simulators * add test cases * add delegating gate test case * update test * all tests pass * add test case for unitary Y * nit * addresses PR comments by adding empty checks. Applys formatter. Subsequent push will add more test cases per Tanuj's comment * nit formatting changes, add docustring with input/output for remove_qubits * merge this branch and tanujkhattar/Cirq@ccde689 * merging branches, adding test coverage in next push * format files * add coverage tests * change assert * coverage and type check tests should pass * incorporate tanujkhattar/Cirq@1db8ac5 * nit * remove block comment * add coverage --------- Co-authored-by: Tanuj Khattar <tanujkhattar@google.com>
This PR addresses #6081 for Density Matrix and State Vector simulators and is related to #6040
It productionizes #6040 (comment) and adds more test cases.