Skip to content
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 tensordot-based method to default.mixed device #3584

Merged
merged 28 commits into from Feb 15, 2023

Conversation

dwierichs
Copy link
Contributor

Context:
The _apply_channel method of default.mixed devices uses qml.math.einsum.
For a device with n wires, einsum does not support applying operations that act on all n wires if n>7, see #3582.

Description of the Change:
This PR adds an _apply_channel_tensordot method to the device, which does not have the above problem.
In addition, preliminary benchmarks showed that the tensordot-based method is equally fast or faster for operations that act on more than a single qubit. Therefore, this PR not only switches the device to use the new tensordot-based method if an operation acts on more than 7 qubits, but already for all multi-qubit operations.

Benefits:
Support of many-qubit operations.
Performance upgrade for multi-qubit operations.

Possible Drawbacks:
A bit of code duplication/complexity. If this is a major drawback, it probably is best to even remove the old einsum-based method, as its performance gain compared to the new method is small even for single-qubit operations.

Related GitHub Issues:
#3582

@codecov
Copy link

codecov bot commented Dec 27, 2022

Codecov Report

Merging #3584 (f45437c) into master (4603d2a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3584   +/-   ##
=======================================
  Coverage   99.80%   99.80%           
=======================================
  Files         331      331           
  Lines       29078    29102   +24     
=======================================
+ Hits        29021    29045   +24     
  Misses         57       57           
Impacted Files Coverage Δ
pennylane/devices/default_mixed.py 100.00% <100.00%> (ø)
pennylane/ops/channel.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@dwierichs dwierichs added review-ready 👌 PRs which are ready for review by someone from the core team. performance ⏲️ Benchmarking and performance improvements labels Dec 27, 2022
@dwierichs dwierichs linked an issue Dec 27, 2022 that may be closed by this pull request
1 task
@ankit27kh
Copy link
Contributor

Hey @dwierichs, I tested this with my original use case, and the results match. Also, this is much faster. For a 10 qubit circuit, einsum implementation took ~120 seconds while tensordot implementation took ~1 second.

Note that the results do not match exactly and differ in 15th or higher digit after the decimal for probability value.

@dwierichs
Copy link
Contributor Author

Hey @ankit27kh, that is great to hear!
Deviations on the 15th decimal place are very common, I'd say, as this is touching the representation precision of float64. As the order and number of floating point operations likely is changed by the patch, I'd be surprised if they would match to even higher precision, where the decimal places basically can be discarded anyways.

@rmoyard rmoyard self-requested a review January 3, 2023 14:27
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks great, nice boost 💯 I have a couple of questions though

pennylane/devices/default_mixed.py Outdated Show resolved Hide resolved
tests/devices/test_default_mixed.py Show resolved Hide resolved
tests/devices/test_default_mixed.py Show resolved Hide resolved
pennylane/devices/default_mixed.py Outdated Show resolved Hide resolved
pennylane/devices/default_mixed.py Outdated Show resolved Hide resolved
pennylane/devices/default_mixed.py Show resolved Hide resolved
tests/devices/test_default_mixed.py Show resolved Hide resolved
@dwierichs
Copy link
Contributor Author

I performed some more comparisons, and the performance of einsum vs tensordot depends heavily on the used interface. As the mixed simulator works for all interfaces, it's difficult to make a decision here. For JAX, for example, the einsum-based approach is fastest for almost all operations and device qubit numbers I considered (up to 6).

@dwierichs
Copy link
Contributor Author

dwierichs commented Jan 27, 2023

I ran a few additional timings (click image for proper size):
apply_channel_variants_times

As we can see, there are massive speedups, but only for interface in [None, "autograd"] and unitary operations that have a single Kraus matrix, and significant slowdowns for other interfaces.
A boundary to decide for tensordot could be num_wires>3 and interface in [None, "autograd"] and len(kraus)>1, but that's already rather specific.

@albi3ro albi3ro added this to the Release v0.29 milestone Feb 13, 2023
pennylane/devices/default_mixed.py Outdated Show resolved Hide resolved
pennylane/devices/default_mixed.py Outdated Show resolved Hide resolved
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks all good, we just need one more test 👍

pennylane/devices/default_mixed.py Show resolved Hide resolved
@rmoyard
Copy link
Contributor

rmoyard commented Feb 14, 2023

Some issues with pylint in the default mixed tests, after that it will be good to be merged 👍 @dwierichs

Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dwierichs It looks good to me 💯

Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@rmoyard rmoyard merged commit 92c7ae7 into master Feb 15, 2023
@rmoyard rmoyard deleted the mixed-apply-tensordot branch February 15, 2023 14:27
mudit2812 pushed a commit that referenced this pull request Apr 13, 2023
* prototype and old tests extension

* additional tests, comparing to matmul. cleanup

* changelog

* use tensordot for all ops with wires > 1

* autodiff compatibility

* changelog: PR number

* clean up

* test readability

* channel tests. skip stack for unitary channels

* commented out tests before merge of #3612

* make matrix conjugation more readable

* early_stack variant

* blcak

* lint tests, activate 3612-related tests

* tests, black, cleanup

* integration tests

* black

* torch stuff

* lint stuff

* solve import problem

* lint

---------

Co-authored-by: Romain Moyard <rmoyard@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance ⏲️ Benchmarking and performance improvements review-ready 👌 PRs which are ready for review by someone from the core team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] too many subscripts in einsum with 8 qubits (default.mixed)
4 participants