-
Notifications
You must be signed in to change notification settings - Fork 583
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
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_gates
works. - Ensure that the empty case works as expected
I went ahead and cleaned up the implementation a little bit. Notable changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for me, but I can't approve my PR. :) @MichaelBroughton
Resolve #603 1st of the three ops.