DefaultTensor: misc scalability improvements #5795
Conversation
vincentmr
changed the title
vincentmr
force-pushed
the
feature/split_expval
branch
from
ed6aadf to
37790fa
Compare
|
Please tag me again when you want a full review :) |
There was a problem hiding this comment.
Great work @vincentmr !
Awesome that we now also have the MPO method 😍 And really cool to see that those gates also work with with the full contraction method.
For the tests in test_scalability.py: is it intentional that they are not asserting / testing anything and just run with 16 qubits? I am not sure that is a good way to test the scalability. Perhaps that is something that we just made sure while implementing this without having it be part of the CI. Instead, we can test that the implemented methods are indeed correct by comparing with exact simulation (at slightly smaller qubit count)?
Co-authored-by: Korbinian Kottmann <43949391+Qottmann@users.noreply.github.com>
Correctness is tested in other modules. The goal here is just make sure things run at all or quickly enough. If the wrong function is hit, it will likely just fail with NumPy complaining it does not have enough memory. |
There was a problem hiding this comment.
Thanks a lot for this @vincentmr! Just a very few comments/questions 👍
|
Also, I just noticed an indentation error in the second example of the documentation for the |
There was a problem hiding this comment.
Thanks again @vincentmr.
I just tried to run a small benchmark on my notebook and the mps method seems a little slower than before (at least with my example, which is the one used in the 'Usage with MPS' section of the doc). In the worst-case scenario, it is a small price to pay to resolve major scalability issues, so this PR has priority.
Finally, I didn't test all the new functionalities and workflows explicitly, but can we currently return the state even with the tn method? In case we can't that's fine with me, but perhaps we should specify this in the documentation. Amazing work!
Laptops don't have very stable timings because they heat so easily. Running the example on an Epyc machine (using 8 threads) I get 0.94 sec. on master and 0.81 sec here. Let's dig more into profiles later on.
Yes, it works for both methods. |
Before submitting
Please complete the following checklist when submitting a PR:
All new features must include a unit test.
If you've fixed a bug or added code that should be tested, add a test to the
test directory!
All new functions and code must be clearly commented and documented.
If you do make documentation changes, make sure that the docs build and
render correctly by running
make docs.Ensure that the test suite passes, by running
make test.Add a new entry to the
doc/releases/changelog-dev.mdfile, summarizing thechange, and including a link back to the PR.
The PennyLane source code conforms to
PEP8 standards.
We check all of our code against Pylint.
To lint modified files, simply
pip install pylint, and thenrun
pylint pennylane/path/to/file.py.When all the above are checked, delete everything above the dashed
line and fill in the pull request template.
Context:
Certain operations and observables cannot be expressed as matrix because of their size, but they have a straightforward MPO form amenable to the MPS simulator.
Description of the Change:
Introduce dispatching for operations and observables.
Introduce method for StateMP.
Handle state preparation in
simulate.Add correctness tests validating gates against DQ.
Add scalability tests.
Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-64793]
[sc-64794]