Add support to apply unitary and non-unitary gates directly to StateVector from Python #121

mlxd · 2021-07-30T09:39:00Z

Context: Previously the implementation allowed application of a given set of gates. This PR allows a user-defined gate to be directly applied to the statevector from Python.

Description of the Change: Bound the C++ method to directly apply matrix in row-major format. Any Python numeric datatype (list of floats, list of complex, numpy array) that match the given index size can be applied directly. Given the size of the gate is minor compared to the size of the statevector, a copy from Python to CPP is applied first. Further architectural changes may be required later to enable reference/direct access without copies, but are beyond this PR scope.

Benefits: Any arbitrary gate can be now called on lightning from Python.

Possible Drawbacks:

Related GitHub Issues: [ch7527]

github-actions · 2021-07-30T09:39:16Z

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

A one-to-two sentence description of the change. You may include a small working example for new features.
A link back to this PR.
Your name (or GitHub username) in the contributors section.

github-actions · 2021-07-30T09:40:02Z

Test Report (C++) on Ubuntu

    1 files ±0     1 suites ±0 0s ⏱️ ±0s
164 tests ±0 164 ✔️ ±0 0 💤 ±0 0 ❌ ±0
694 runs ±0 694 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 800eaf3. ± Comparison against base commit 800eaf3.

♻️ This comment has been updated with latest results.

codecov · 2021-07-30T09:40:59Z

Codecov Report

Merging #121 (f77db2e) into master (e309d44) will not change coverage.
The diff coverage is n/a.

❗ Current head f77db2e differs from pull request most recent head 132eb49. Consider uploading reports for the commit 132eb49 to get more accurate results

@@            Coverage Diff            @@
##            master      #121   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           54        54           
=========================================
  Hits            54        54

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e309d44...132eb49. Read the comment docs.

…lightning into 7527_unitary_gate

trbromley

Thanks @mlxd! Looks great and I'm really excited to get this feature finally in 🎉

I've left some comments but will defer to @trevor-vincent's feedback for a deeper dive on the technical side.

trbromley · 2021-08-03T19:58:20Z

CMakeLists.txt

-if ("$ENV{USE_LAPACK}" OR "${USE_LAPACK}")
-    message(STATUS "Use LAPACKE")
-    target_link_libraries(external_dependency INTERFACE lapacke)
-    target_compile_options(external_dependency INTERFACE "-DLAPACKE=1")
-endif()
-


Why this change?

We make no use of LAPACK functions or methods in the library right now, so we don't require this. If you think keeping this to a separate PR is better I'm fine with that.

trbromley · 2021-08-03T20:10:35Z

pennylane_lightning/src/StateVector.hpp

+     */
+    void applyOperation(const std::vector<CFP_t> &matrix,
+                        const vector<size_t> &wires, bool inverse = false,
+                        [[maybe_unused]] const vector<fp_t> &params = {}) {


Agreed, I can't see why we'd use it here if we have the matrix directly.

Yea, the params here is to allow simplified dispatch functionality. We define std::function with a given signature elsewhere, and attempt to ensure similarity throughout. Keeping the params in there was just a cheap way of ensuring this with no additional overheads, since [[maybe_unused]] is now fully supported by the standard.

trbromley · 2021-08-03T20:15:13Z

pennylane_lightning/src/StateVector.hpp

+                if (inverse == true) {
+                    for (size_t j = 0; j < indices.size(); j++) {
+                        const size_t baseIndex = j * indices.size();
+                        shiftedState[index] +=
+                            conj(matrix[baseIndex + i]) * v[j];
+                    }


This is great if the matrix is a unitary, but it will not be the inverse of an arbitrary matrix 🤔 I guess we'll assume unitarity is checked Python-side and inverse can never be set to true if non-unitary?

Yea, that was something I was worried about --- I think the safety checks happening Python side make sense for now. If non unitary always ensure inverse is false sounds good for now.

pennylane_lightning/src/Util.hpp

pennylane_lightning/src/tests/TestHelpers.hpp

trbromley · 2021-08-03T20:32:44Z

pennylane_lightning/src/Bindings.cpp

+     * @param matrix
+     * @param wires
+     */
+    void applyMatrixWires(const std::vector<std::complex<fp_t>> &matrix,


Agree that for this PR it is ok to go for a copy of the matrix rather than passing by reference, but do you have an idea of what changes would be needed to support passing by reference? Would it be a major overhaul?

Isn't the above signature passing by reference already?

trbromley · 2021-08-03T20:35:42Z

pennylane_lightning/src/Bindings.cpp

+    void applyMatrixWires(const std::vector<std::complex<fp_t>> &matrix,
+                          const vector<size_t> &wires, bool inverse = false) {
+        this->applyOperation(matrix, wires, inverse);
+    }
+    /**
+     * @brief Directly apply a given matrix to the specified wires. Data in 1/2D
+     * numpy complex array format.
+     *
+     * @param matrix
+     * @param wires
+     * @param inverse
+     */
+    void applyMatrixWires(const py::array_t<complex<fp_t>> &matrix,
+                          const vector<size_t> &wires, bool inverse = false) {


For the two cases of applyMatrixWires, will the second one be the most commonly used (at least initially)? I.e., with directly passing the numpy array. Is there an advantage of the first approach and do we want to steer towards that down the line?

Yea, that is the plan. The second approach (the py::array_t version) works with the way we have everything currently implemented, and allows for as easy as possible a conversion from the numpy array to the C++ layer by casting the pointer to the data buffer, an following through to the underlying implementation accepting pointers over references. The first approach though will be easier to maintain, and adapt. We have both now as both will allow development and comparison to define our needs as the CPP implementations are fleshed out more.

For the moment I think this is the best way for us to handle this, but long-term, I think we may opt to have a seamless CPP backed matrix class that we pass instead. This would alleviate the need for the multiple overloads, but may make interop with other python interfaces a little trickier.

I think this will likely evolve in time once we eventually move away from the fully Python defined implementations.

trbromley · 2021-08-03T20:41:44Z

pennylane_lightning/src/Bindings.cpp

+    void apply(const vector<string> &ops, const vector<vector<size_t>> &wires,
+               const vector<bool> &inverse) {
+        this->applyOperations(ops, wires, inverse);
+    }


Is this for the case where we want to pass multiple operations that do not have parameters? E.g., CNOTs, Toffolis, etc?
What is the advantage of this approach vs just passing params as []?

Yea, you are right about the nonparam gates. I added this to simply user-experience if choosing to apply defined gates, as I don't think we should require the user to apply empty params if there are no parametric gates needed. Having an overload for this seemed like the best call, though I can remove this and refer to the empty param args if you think it best.

trbromley · 2021-08-03T20:44:09Z

pennylane_lightning/src/Bindings.cpp

+        .def("apply",
+             py::overload_cast<
+                 const vector<string> &, const vector<vector<size_t>> &,
+                 const vector<bool> &, const vector<vector<PrecisionT>> &>(
+                 &StateVecBinder<PrecisionT>::apply))
+        .def("apply", py::overload_cast<const vector<string> &,


In the long run, is the plan to expose applyOperation and have the Python side iterate through the operations?

I think this is an open question. Whether we aim to have Python iterate through the supplied operations and directly call the newly defined class methods, or simply offload everything to the CPP layer is something I think we eventually should check. Given some conversions are required to pass between the interfaces, my gut says to let Python handle it and make direct calls there, but we should leave this to figure out at a later date once up and running.

trevor-vincent

Awesome work Lee! I didn't see any issues beyond what Tom posted.

mlxd added 30 commits July 14, 2021 13:00

Initial reformat and templated of gate tests

175e9b2

Redefine gate mappings with templates

ae8d7c3

Record given state

b7d5106

Move all gate classes to become StateVector methods

85fe333

Fix compile errors for SV

dc70228

Refactor gates implementation

6a4fa76

Enable vector param methods

6d85893

Enable label to gatename map

dad9c8a

Support dispatch directly from StateVector class

0d6af2f

Add apply methods to SV class

e157b99

Add log2 utility function

3286f40

Remove old file arch

fe7bf57

Ensure bindings build support for float and double sizes complex data

2c0e318

Remove outdated tests for deleted modules

cdd7814

Remove old definition headers

5da9487

Tidy dispatch map

2ad4801

Replace header with correct type

88a5341

Ensure cpp17 is now used

f3e4c49

Remove unneeded files in compilation

8c03920

Ensure apply args are given in correct order

6318cc5

Allow make test to be run from working dir without cleaning

cc06018

Fix cpp formatting

b608595

Merge branch 'master' into 7067-templated-float

6652244

Remove old code and fix codefactor complaints

09b786b

Add cast to enable wheel build on MacOS

810c52f

Enable MSVC intrinsics if using Windows

a90d9a9

Ensure C++17 as a requirement

a7c80ea

Fix compile-time ifdef

f627831

Replace intrinsic with BSR

f90f915

Avoid intrinsics for portability

9a5c566

mlxd added 9 commits July 27, 2021 17:29

Add testing and safety checks for utilities

fb9052f

Remove LAPACK from build system

16569e6

Add placeholder functions for gate multiplication

681321f

Format TestHelpers

3cd1df3

Format utility tests

8a4f4a1

Add unitary apply to bindings

702604a

Merge branch 'master' into 7527_unitary_gate

a2d93b5

Remove unneeded functions

45d7563

Relabel matrix applicator methods

517f8fd

Merge branch 'master' into 7527_unitary_gate

601415d

mlxd added 3 commits July 30, 2021 11:16

Appease the codefactor

e31b6dd

Merge branch '7527_unitary_gate' of github.com:PennyLaneAI/pennylane-…

6fcceaa

…lightning into 7527_unitary_gate

Split tests into param and nonparam

9c4ab63

mlxd requested review from trbromley and trevor-vincent and removed request for trbromley July 30, 2021 10:42

Add support for numpy 2D array gate data without copies

ed8e195

mlxd requested a review from trbromley July 30, 2021 11:07

Appease codefactor

f191eae

trbromley reviewed Aug 3, 2021

View reviewed changes

trevor-vincent approved these changes Aug 9, 2021

View reviewed changes

mlxd and others added 4 commits August 9, 2021 11:37

Offload dim check to util function

9eeae42

Add docstring for utility functions

6f8aef6

Codefactor fixes

49b3442

Merge branch 'master' into 7527_unitary_gate

132eb49

mlxd merged commit 800eaf3 into master Aug 9, 2021

mlxd deleted the 7527_unitary_gate branch August 9, 2021 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to apply unitary and non-unitary gates directly to StateVector from Python #121

Add support to apply unitary and non-unitary gates directly to StateVector from Python #121

mlxd commented Jul 30, 2021

github-actions bot commented Jul 30, 2021

github-actions bot commented Jul 30, 2021 •

edited

Loading

codecov bot commented Jul 30, 2021 •

edited

Loading

trbromley left a comment

trbromley Aug 3, 2021

mlxd Aug 9, 2021

trbromley Aug 3, 2021

mlxd Aug 9, 2021

trbromley Aug 3, 2021

mlxd Aug 9, 2021

trbromley Aug 3, 2021

mlxd Aug 9, 2021

trbromley Aug 3, 2021

mlxd Aug 9, 2021

trbromley Aug 3, 2021

mlxd Aug 9, 2021

trbromley Aug 3, 2021

mlxd Aug 9, 2021

trevor-vincent left a comment

Add support to apply unitary and non-unitary gates directly to StateVector from Python #121

Add support to apply unitary and non-unitary gates directly to StateVector from Python #121

Conversation

mlxd commented Jul 30, 2021

github-actions bot commented Jul 30, 2021

github-actions bot commented Jul 30, 2021 • edited Loading

Test Report (C++) on Ubuntu

codecov bot commented Jul 30, 2021 • edited Loading

Codecov Report

trbromley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevor-vincent left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 30, 2021 •

edited

Loading

codecov bot commented Jul 30, 2021 •

edited

Loading