Use templates to simplify applying operations to tensors #52

ThomasLoke · 2021-01-02T08:12:35Z

Also extends support from 16 qubits to 50 qubits (its unlikely anyone would have memory on the order of petabytes).

codecov · 2021-01-02T08:22:40Z

Codecov Report

Merging #52 (3171839) into master (2eac157) will not change coverage.
The diff coverage is 100.00%.

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

Impacted Files	Coverage Δ
pennylane_lightning/lightning_qubit.py	`100.00% <100.00%> (ø)`

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 2eac157...3171839. Read the comment docs.

josh146 · 2021-01-04T04:22:38Z

Thanks @ThomasLoke for this contribution! Someone from the team will get back to you shortly with a code review

antalszava

Hi @ThomasLoke, this is a really exciting improvement, thank you so much! 😍

The code looks way neater! 🎊

Two things stood out as further changes needed:

Updating the Python wrapper to allow >16 qubits
Adjusting a couple of docstrings

The rest of the comments are minor suggestions/questions for my own understanding.

antalszava · 2021-01-04T23:09:50Z

pennylane_lightning/src/typedefs.hpp

@@ -0,0 +1,26 @@
+#pragma once


Might be worth adding the license and a description of the module (see pennylane_lightning/src/lightning_qubit.hpp as an example).

Suggested change

#pragma once

// Copyright 2020 Xanadu Quantum Technologies Inc.

// Licensed under the Apache License, Version 2.0 (the "License");

// you may not use this file except in compliance with the License.

// You may obtain a copy of the License at

// http://www.apache.org/licenses/LICENSE-2.0

// Unless required by applicable law or agreed to in writing, software

// distributed under the License is distributed on an "AS IS" BASIS,

// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

// See the License for the specific language governing permissions and

// limitations under the License.

#pragma once

antalszava · 2021-01-04T23:11:10Z

pennylane_lightning/src/typedefs.hpp

+using pfunc_Xq_one_param = Gate_Xq<X>(*)(const double&);
+
+template<int X>
+using pfunc_Xq_three_params = Gate_Xq<X>(*)(const double&, const double&, const double&);


Suggested change

using pfunc_Xq_three_params = Gate_Xq<X>(*)(const double&, const double&, const double&);

using pfunc_Xq_three_params = Gate_Xq<X>(*)(const double&, const double&, const double&);

I've never been a fan of an extra line of whitespace at the end of a file. I don't mind adding it, but I thought I'd ask why the preference?

Apparently it's just a POSIX convention: https://stackoverflow.com/a/729795/10958457

But it's such a strong convention these days, that all linters will complain if it is missing (even github lol 😆 )

Aha, fair enough! I guess I'll modify my preferences then :)

antalszava · 2021-01-04T23:12:01Z

pennylane_lightning/src/lightning_qubit.hpp

+    {
+        return QubitOperationsGenerator<Dim, Dim>::apply(state, ops, wires, params);
+    }
+};


Suggested change

};

};

antalszava · 2021-01-04T23:15:30Z

pennylane_lightning/src/lightning_qubit.cpp

+    case 48: return QubitOperations<48>::apply(state, ops, wires, params);
+    case 49: return QubitOperations<49>::apply(state, ops, wires, params);
+    case 50: return QubitOperations<50>::apply(state, ops, wires, params);
+    default: throw std::invalid_argument("No support for > 50 qubits");


💯

With the PennyLane UI this is checked in Python, see

pennylane-lightning/pennylane_lightning/lightning_qubit.py

Line 87 in 2eac157

_MAX_WIRES = 16

The max number of wires will have to be updated there too.

antalszava · 2021-01-04T23:17:44Z

pennylane_lightning/src/lightning_qubit.cpp

-* Note that only up to 16 qubits are currently supported. This limitation is due to the Eigen
-* Tensor library not supporting dynamically ranked tensors.
-*


Maybe worth keeping this but with 50?

antalszava · 2021-01-04T23:42:48Z

pennylane_lightning/src/lightning_qubit.hpp

+        const vector<string>& ops,
+        const vector<vector<int>>& wires,
+        const vector<vector<float>>& params,
+        Values... values)


(Minor suggestion): just to conform to with the apply_ops function signature. If accepted, further applies in the signatures below.

Suggested change

Values... values)

Shape... shape)

antalszava · 2021-01-04T23:49:26Z

pennylane_lightning/src/lightning_qubit.hpp


    return Map<VectorXcd> (shuffled_evolved_tensor.data(), shuffled_evolved_tensor.size(), 1);
 }
+
+// Template classes for a generic interface for qubit operations


It might be good to make a class docstring for the generic class (which can be referenced by specialized classes). The docstring could also include the description of template parameters (this is not used in other docstrings in the code base right now, but would actually be useful to be adapted).

antalszava · 2021-01-04T23:55:23Z

pennylane_lightning/src/lightning_qubit.hpp

+        const vector<vector<float>>& params,
+        Values... values)
+    {
+        return QubitOperationsGenerator<Dim, ValueIdx - 1>::apply(state, ops, wires, params, 2, values...);


Would we want to restrict the ranges of Dim and ValueIdx? E.g., what happens for ValueIdx==0?

The ValueIdx==0 case is handled by the specialisation:

// Generic multi-qubit case template<int Dim> class QubitOperationsGenerator<Dim, 0>

which is how the recursive template terminates. I could add a runtime check here, but it seems unnecessary especially since it's going to be checked N times if ValueIdx starts at N due to recursion. Unless there's a way of statically enforcing ValueIdx >= 0?

Right! Was curious about the ValueIdx==0 and return QubitOperationsGenerator<Dim, -1>::apply(state, ops, wires, params, 2, values...); case (and further negative values as the second template argument).

it seems unnecessary especially since it's going to be checked N times if ValueIdx starts at N due to recursion

Yeah, the current way of using it wouldn't result in ValueIdx <1. Was curious what would happen if such a ValueIdx is inputted explicitly.

Unless there's a way of statically enforcing ValueIdx >= 0?

Hmm, think a runtime check would be applicable in such a case. 🤔

Was curious what would happen if such a ValueIdx is inputted explicitly.

Because templates are all determined at compile-time, I believe it'll recurse up until the compiler's template recursion limit, at which point it'll fail to compile. So in theory, it shouldn't even compile.

Of course, we might at some point hit the template recursion limit for really large numbers of qubits. At least on MSVC, its larger than 50. If we do cross that bridge, a more scalable approach will be needed that doesn't involve Eigen tensors with an arbitrary rank.

That's right, just checked it and it indeed runs into compile-time problems. Thanks for the clarification!

antalszava · 2021-01-04T23:56:34Z

pennylane_lightning/src/typedefs.hpp

+template<int X>
+using State_Xq = Eigen::Tensor<std::complex<double>, X>;
+
+template<int X>
+using Gate_Xq = Eigen::Tensor<std::complex<double>, 2 * X>;
+
+using Pairs = Eigen::IndexPair<int>;
+template<int X>
+using Pairs_Xq = std::array<Eigen::IndexPair<int>, X>;
+
+// Creating aliases based on the function signatures of each operation
+
+template<int X>
+using pfunc_Xq = Gate_Xq<X>(*)();
+
+template<int X>
+using pfunc_Xq_one_param = Gate_Xq<X>(*)(const double&);
+
+template<int X>
+using pfunc_Xq_three_params = Gate_Xq<X>(*)(const double&, const double&, const double&);


antalszava · 2021-01-04T23:57:37Z

pennylane_lightning/src/lightning_qubit.cpp

+    if (qubits <= 0)
+        throw std::invalid_argument("Must specify one or more qubits");
+


(Minor suggestion): could add a small unit test here and for qubits > 50.

Thought about this, but I didn't feel like it was worth the hassle of maintaining (especially trying to fake a 50+ qubit invocation of the method; I could just put invalid parameters in, but I wouldn't blame the test for failing for any other reason).

ThomasLoke · 2021-01-05T01:31:31Z

Updating the Python wrapper to allow >16 qubits

🤦 My mistake, I forgot to test the Python side of things. I'll address that and the other review changes.

ThomasLoke · 2021-01-06T18:26:55Z

Failing test:

self = <LightningQubit device (wires=51, shots=1000) at 0x7fdfd4480350>
...
numpy.core._exceptions._ArrayMemoryError: Unable to allocate 32.0 PiB for an array with shape (2251799813685248,) and data type complex128

Which is unsurprising. Not sure what I should do with this--remove the test?

Also I've not worked with Black before--what did I miss?

chaserileyroberts · 2021-01-06T18:48:17Z

Failing test:

LightningQubit device wires=51

Unable to allocate 32.0 PiB

for an array with shape 2251799813685248

Which is unsurprising.

It's reads like a poem 😆

antalszava · 2021-01-07T02:48:37Z

Which is unsurprising. Not sure what I should do with this--remove the test?

After importing import pennylane_lightning in the test file, this should do it:

    def test_warning(self, monkeypatch):
        """Test that a warning is produced when lightning.qubit is instantiated with more than
        the supported number of qubits"""

        class MockDefaultQubit:
            """Mock DefaultQubit class."""

            def __init__(self, wires, *args, **kwargs):
                """Mock __init__ method."""
                self.num_wires = wires

        with monkeypatch.context() as m:
            # Mock the DefaultQubit class to avoid an extensive memory allocation
            m.setattr(pennylane_lightning.lightning_qubit.DefaultQubit, "__init__", MockDefaultQubit.__init__)
            with pytest.warns(UserWarning, match="The number of wires exceeds"):
                pennylane_lightning.lightning_qubit.LightningQubit(wires=LightningQubit._MAX_WIRES + 1)

Also I've not worked with Black before--what did I miss?

The tool uses black-20.8b1, which version do you have locally? Running black -l 100 pennylane_lightning/lightning_qubit.py would do the reformatting.

josh146 · 2021-01-07T06:42:57Z

It's reads like a poem 😆

like an accidental haiku

antalszava · 2021-01-11T18:32:46Z

pennylane_lightning/src/lightning_qubit.hpp

+* Main recursive template to generate multi-qubit operations
+* @tparam Dim The number of qubits (i.e. tensor rank)
+* @tparam ValueIdx Index to be decremented recursively until 0 to generate the dimensions of the tensor


Suggested change

* Main recursive template to generate multi-qubit operations

* @tparam Dim The number of qubits (i.e. tensor rank)

* @tparam ValueIdx Index to be decremented recursively until 0 to generate the dimensions of the tensor

* Main recursive template to generate multi-qubit operations.

*

* @tparam Dim the number of qubits (i.e. tensor rank)

* @tparam ValueIdx index to be decremented recursively until 0 to generate the dimensions of the tensor

antalszava · 2021-01-11T18:33:38Z

pennylane_lightning/src/lightning_qubit.hpp

+* Terminal specialised template for general multi-qubit operations
+* @tparam Dim The number of qubits (i.e. tensor rank)


Suggested change

* Terminal specialised template for general multi-qubit operations

* @tparam Dim The number of qubits (i.e. tensor rank)

* Terminal specialised template for general multi-qubit operations.

*

* @tparam Dim the number of qubits (i.e. tensor rank)

antalszava · 2021-01-11T18:34:17Z

pennylane_lightning/src/lightning_qubit.hpp

+* Terminal specialised template for single qubit operations
+* @tparam ValueIdx Ignored, but required to specialised the main recursive template


Suggested change

* Terminal specialised template for single qubit operations

* @tparam ValueIdx Ignored, but required to specialised the main recursive template

* Terminal specialised template for single qubit operations.

*

* @tparam ValueIdx ignored but required to specialise the main recursive template

antalszava · 2021-01-11T18:34:36Z

pennylane_lightning/src/lightning_qubit.hpp

+* Terminal specialised template for two qubit operations
+* @tparam ValueIdx Ignored, but required to specialised the main recursive template


Suggested change

* Terminal specialised template for two qubit operations

* @tparam ValueIdx Ignored, but required to specialised the main recursive template

* Terminal specialised template for two qubit operations.

*

* @tparam ValueIdx ignored but required to specialise the main recursive template

antalszava · 2021-01-11T18:34:52Z

pennylane_lightning/src/lightning_qubit.hpp

+* Generic interface that invokes the generator to generate the desired multi-qubit operation
+* @tparam Dim The number of qubits (i.e. tensor rank)


Suggested change

* Generic interface that invokes the generator to generate the desired multi-qubit operation

* @tparam Dim The number of qubits (i.e. tensor rank)

* Generic interface that invokes the generator to generate the desired multi-qubit operation.

*

* @tparam Dim the number of qubits (i.e. tensor rank)

antalszava · 2021-01-11T18:38:27Z

pennylane_lightning/src/typedefs.hpp

+// Copyright 2020 Xanadu Quantum Technologies Inc.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//     http://www.apache.org/licenses/LICENSE-2.0
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.


Worth adding the whitespace as for other files.

Suggested change

// Copyright 2020 Xanadu Quantum Technologies Inc.

// Licensed under the Apache License, Version 2.0 (the "License");

// you may not use this file except in compliance with the License.

// You may obtain a copy of the License at

// http://www.apache.org/licenses/LICENSE-2.0

// Unless required by applicable law or agreed to in writing, software

// distributed under the License is distributed on an "AS IS" BASIS,

// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

// See the License for the specific language governing permissions and

// limitations under the License.

// Copyright 2020 Xanadu Quantum Technologies Inc.

// Licensed under the Apache License, Version 2.0 (the "License");

// you may not use this file except in compliance with the License.

// You may obtain a copy of the License at

// http://www.apache.org/licenses/LICENSE-2.0

// Unless required by applicable law or agreed to in writing, software

// distributed under the License is distributed on an "AS IS" BASIS,

// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

// See the License for the specific language governing permissions and

// limitations under the License.

antalszava · 2021-01-11T18:39:59Z

pennylane_lightning/lightning_qubit.py

+                "The number of wires exceeds {}, reverting to NumPy-based evaluation.".format(
+                    self._MAX_WIRES
+                ),


Could use an f string here

Suggested change

"The number of wires exceeds {}, reverting to NumPy-based evaluation.".format(

self._MAX_WIRES

),

f"The number of wires exceeds {self._MAX_WIRES}, reverting to NumPy-based evaluation."

antalszava

Looks good to me! 💯 This is a great change @ThomasLoke, thank you! 🙂

Left some minor suggestions, once they are addressed this is merge-ready from my side.

trbromley

Thanks @ThomasLoke!

ThomasLoke · 2021-01-12T17:08:27Z

Thanks everyone ^_^

ThomasLoke added 4 commits January 2, 2021 15:47

Generic template interface for qubit operations

ab31ca0

Move typedefs into a separate header file + use templates to extend them

bee8b28

Extend support to 50 qubits

c82e2a2

Update compilation

6e8c52e

josh146 requested review from trbromley and antalszava January 4, 2021 04:22

antalszava suggested changes Jan 5, 2021

View reviewed changes

ThomasLoke added 2 commits January 6, 2021 23:38

Review changes: Bump up maximum number of qubits from 16 to 50 in python

5d128b1

Review changes: Misc

1641bd5

ThomasLoke added 3 commits January 10, 2021 16:53

Review changes: Black formatting

344d10c

Review change: Amend test case

073d0e9

Add missing import

4f4166a

antalszava reviewed Jan 11, 2021

View reviewed changes

antalszava approved these changes Jan 11, 2021

View reviewed changes

ThomasLoke added 2 commits January 12, 2021 08:45

Review changes: Formatting

fa04ced

Review changes: f string

3171839

trbromley approved these changes Jan 12, 2021

View reviewed changes

antalszava merged commit 14b50f9 into PennyLaneAI:master Jan 12, 2021

	using pfunc_Xq_three_params = Gate_Xq<X>(*)(const double&, const double&, const double&);
	using pfunc_Xq_three_params = Gate_Xq<X>(*)(const double&, const double&, const double&);

		if (qubits <= 0)
		throw std::invalid_argument("Must specify one or more qubits");

		* Terminal specialised template for general multi-qubit operations
		* @tparam Dim The number of qubits (i.e. tensor rank)

		* Terminal specialised template for single qubit operations
		* @tparam ValueIdx Ignored, but required to specialised the main recursive template

		* Terminal specialised template for two qubit operations
		* @tparam ValueIdx Ignored, but required to specialised the main recursive template

		* Generic interface that invokes the generator to generate the desired multi-qubit operation
		* @tparam Dim The number of qubits (i.e. tensor rank)

Use templates to simplify applying operations to tensors #52

Use templates to simplify applying operations to tensors #52

Conversation

ThomasLoke commented Jan 2, 2021 • edited Loading

codecov bot commented Jan 2, 2021 • edited Loading

Codecov Report

josh146 commented Jan 4, 2021

antalszava 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

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

ThomasLoke commented Jan 5, 2021

ThomasLoke commented Jan 6, 2021 • edited Loading

chaserileyroberts commented Jan 6, 2021

antalszava commented Jan 7, 2021 • edited Loading

josh146 commented Jan 7, 2021

antalszava Jan 11, 2021 • edited Loading

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

antalszava left a comment

Choose a reason for hiding this comment

trbromley left a comment

Choose a reason for hiding this comment

ThomasLoke commented Jan 12, 2021

ThomasLoke commented Jan 2, 2021 •

edited

Loading

codecov bot commented Jan 2, 2021 •

edited

Loading

ThomasLoke commented Jan 6, 2021 •

edited

Loading

antalszava commented Jan 7, 2021 •

edited

Loading

antalszava Jan 11, 2021 •

edited

Loading