Update MPS expectation value snapshots #509
Conversation
yaelbh
changed the title
|
This pull request works fine for the Pauli expectation value but not for the matrix expectation value. I've removed the test of the matrix expectation value, so all tests should pass now, and the pull request is ready for review. @chriseclectic could you please make it to the upcoming release. It is important because it allows faster simulation of VQE instances. |
|
I see that tests are failing. They pass for me locally. Checking. |
|
Apparently there is a core dump in |
| @@ -75,6 +77,8 @@ class TestQasmMatrixProductStateSimulator( | |||
| QasmSnapshotStatevectorTests, | |||
| QasmSnapshotProbabilitiesTests, | |||
| QasmSnapshotStabilizerTests | |||
| #QasmSnapshotExpValPauliTests, | |||
There was a problem hiding this comment.
What is the issue with the failing tests? Is it due to a bug in the expectation value method or something else? We need the ExpVal tests to pass to merge, otherwise we can't trust the results.
There was a problem hiding this comment.
There are actually 3 issues in this PR:
- bug in the interface of MPS expectation value, following the addition of SnapshotDataType. This was fixed by Yael.
- bug in Pauli expectation value when
qubitsparameter is not given in ascending order. - bug in matrix expectation value when
qubitsparameter is not given in ascending order.
These were discovered because Yael added new tests after her change.
I am in the process of fixing 2 and 3, but would rather address them as separate issues (I will open these). The interface change (1) is more urgent, because that enables the users to use MPS in VQE. I stress that when the qubits are given in the default order, the expectation value works fine.
|
Superseed by #619 |
Summary
Updated MPS expectation value snapshots to support snapshots with variance and single shot.
Details and comments
@merav-aharoni can you please check a failing test.