Add tfq.math.mps_1d_expectation for 1D MPS#610
Conversation
tfq.math.1d_expectation for 1D MPStfq.math.mps_1d_expectation for 1D MPS
MichaelBroughton
left a comment
There was a problem hiding this comment.
This is a great first pass @jaeyoo ! Would you be able to add some tests that verify expectations values computed using cirq's built in expectation calculation functions ? Also removing some of the print statements from the code :)
Both Operation with natural born controlled gate (e.g. CNOT) and synthesized ControlledOperation are converted into the same tfq_gate_set with control_qubit. So, we can't filter ControlledOperation in this way, revert the previous commit.
|
@jaeyoo any updates on this ? (My workstation has gone offline and won't be into the office for a while to turn it back on). |
I am still working on how to fix the |
|
@MichaelBroughton I could fix the error finally. It was not easy because it had multiple reasons.
Thank you so so much for your kind patience. |
|
Failed at //tensorflow_quantum/core/ops/noise:noisy_samples_op_test May i just re-run the test? |
|
PTAL Note: |
|
We just cut a release here: https://github.com/quantumlib/qsim/releases/tag/v0.10.3-dev%2B20211001 Could we try upgrading to this new qsim version ? |
Sure, it's my pleasure. Done. |
MichaelBroughton
left a comment
There was a problem hiding this comment.
Looking great! Just a few moderate sized changes then we should be good to merge!
| bond_dim: `tf.Tensor` for an integer representing bond dimension | ||
| in this 1D MPS. This will create the following MPS: | ||
| [2, bond_dim], [bond_dim, 2, bond_dim] ... [bond_dim, 2] | ||
|
|
||
| The `bond_dim` should be >= 4. |
There was a problem hiding this comment.
Wouldn't this just be a constant and doesn't have to be a tf.Tensor ?
| if (max_num_qubits >= 26 || programs.size() == 1) { | ||
| ComputeLarge(num_qubits, fused_circuits, pauli_sums, context, | ||
| &output_tensor); |
There was a problem hiding this comment.
Since MPS simulation doesn't have such a massive memory footprint as statevector I think we don't need ComputeLarge anymore.
| util.convert_to_tensor([[x] for x in pauli_sums])) | ||
| self.assertDTypeEqual(res, np.float32) | ||
|
|
||
| def test_simulate_mps_1d_expectation_results(self): |
There was a problem hiding this comment.
This is a good start. Could we add a few more tests (with different names) here that do the following:
- Test with all single qubit gates in
tfq.get_supported_gates - Test with all single qubit gates in
tfq.get_supported_gates.controlled_by(a_qubit)and ensure that this fails or passes accordingly) - Test that all two qubit gates in
tfq.get_supported_gatesworks. - Ensure that the empty case works as expected
|
I went ahead and cleaned up the implementation a little bit. Notable changes:
|
jaeyoo
left a comment
There was a problem hiding this comment.
LGTM for me, but I can't approve my PR. :) @MichaelBroughton
Resolve #603 1st of the three ops.