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

LM implementation of multi-wire gates #358

Closed
wants to merge 295 commits into from
Closed

Conversation

multiphaseCFD
Copy link
Member

@multiphaseCFD multiphaseCFD commented Sep 21, 2022

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
    tests 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 .github/CHANGELOG.md file, summarizing the
    change, and including a link back to the PR.

  • Ensure that code is properly formatted by running make format.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context:
This PR add LM implementation for 3 Qubits gates (CSWAP & Toffoli) and QChem 4 Qubits gates (DoubleExcitation, DoubleExcitationMinus, DoubleExcitationPlus).
Description of the Change:
Add LM implementation for 3 Qubits gates (CSWAP & Toffoli) and QChem 4 Qubits gates (DoubleExcitation, DoubleExcitationMinus, DoubleExcitationPlus).
Benefits:
The speed of LM implementation of CSWAP, Toffoli and DoubleExcitation gates is faster than their PI implementation for all tests number of qubits. Per DoubleExcitationMinus and DoubleExcitationPlus gates, LM implementation is faster than PI implementation for small number qubit systems (<10 qubits), while slower than PI implementation for larger number of qubit systems (>10 qubits).

Possible Drawbacks:

Related GitHub Issues:
#315

antalszava and others added 30 commits July 22, 2020 11:05
* Adding new operations file; including in test file; updating Makefile to include pybind path

* Adding Paulis and tests

* Adding using directives; removing pybind dependence from Makefile and operations.hpp

* Renaming test cases

* Apply suggestions, change naming conventions

* Adding Hadamard

* Adding S, T and rotation gates

* Update pennylane_lightning/src/operations.hpp

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane_lightning/src/operations.hpp

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Adding CNOT

* Changing local var name for T

* Update pennylane_lightning/src/tests/lightning_unittest.cpp

Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Update pennylane_lightning/src/tests/lightning_unittest.cpp

Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Update pennylane_lightning/src/tests/lightning_unittest.cpp

Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Update pennylane_lightning/src/tests/lightning_unittest.cpp

Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Transition to using expect_true

* Applying Nathan's suggestions on naming from code review

* Creating aliases for 1-3 qubit states; using these in the tests

* Adding test for three qubit CNOT where the control is the third qubit; correcting 3q rank

* updating CNOT test

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
Adds skeleton for C++ section of lightning.qubit
* factor out permutations

* Remove hadamard and add in operations header

* Update ops

* 1q gate addition

* add gates

* Get things closer to working

* Get rid of column major

* Add Rot gate

* Add Rot properly

* add to python side

* Update parameter access

* Update

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
* factor out permutations

* Remove hadamard and add in operations header

* Update ops

* 1q gate addition

* add gates

* Get things closer to working

* Get rid of column major

* Add Rot gate

* Add Rot properly

* add to python side

* Update parameter access

* Update

* Create maps for ops

* Maps for 1 qubit 3 params; polishing

* Extracting the maps out of the functions; declaring them constant; turning to using map.at (constant function)

* Moving map defs to header

* Remove extern keyword for constants (they are defined too, not just declared)

* Moving declarations into if statements

* Adding else if so that the second if statement gets executed only if the first was false

Co-authored-by: trbromley <brotho02@gmail.com>
* Partially add multiple qubits

* Nearly working

* Add argsort again

* multiqubit version

* Remove explicit pass of qubit
* Partially add multiple qubits

* Nearly working

* Add argsort again

* multiqubit version

* Adding Rot tests

* Remove unnecessary tests

* Consts

* Perm tests

* Argsort tests

* Adding basic build.yml

* Remove libomp

* Installing gtest

* Add building and running tests

* Correct command format

* Running testing with higher permissions

* Changing the path

* Remove being triggered on push

* Adjustments for the integration test suite from PL

* Try without sudo

* Simplify permutation tests by checking for equality between the expected and the output vectors

* Fix a failing test:

Co-authored-by: trbromley <brotho02@gmail.com>
* First addition of docs

* Update README

* Update

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Make changes from code review

* Add supported ops and obs

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
* Partially add multiple qubits

* Nearly working

* Add argsort again

* multiqubit version

* Add CRot

* Add CRZ

* Add CRY

* Add target gates

* Add CRX

* Add CRX

* Add phaseshift

* Add CZ

* add toffoli

* Decide not to refactor

* Merge better

* Add SWAP

* Add CSWAP

* Don't support MultiRZ

* Don't support arbitrary unitaries

* Update gitignore

* Tests not working

* Refactor

* Support multiple qubit numbers

* Support multiple qubit numbers

* Update lightning_qubit.py

* Change iterator type

* Add CRX etc tests

* Add Rot to tests

* Remove extra file

* Remove extras

* Merge properly

* Update

* Add PhaseShift test

* Fix bug

* Remove O3 flag

* Add Toffoli test

* Add CSWAP test

* Update qubit number restriction

* Remove params from 3q gates:

* Apply suggestions from code review

* Make Rot tests clearer

* Add vectorize() function
* Partially add multiple qubits

* Nearly working

* Add argsort again

* multiqubit version

* Add CRot

* Add CRZ

* Add CRY

* Add target gates

* Add CRX

* Add CRX

* Add phaseshift

* Add CZ

* add toffoli

* Decide not to refactor

* Merge better

* Add SWAP

* Add CSWAP

* Don't support MultiRZ

* Don't support arbitrary unitaries

* Update gitignore

* Tests not working

* Refactor

* Support multiple qubit numbers

* Support multiple qubit numbers

* Update lightning_qubit.py

* Change iterator type

* Add CRX etc tests

* Add Rot to tests

* Remove extra file

* Remove extras

* Merge properly

* Update

* Add PhaseShift test

* Fix bug

* Remove O3 flag

* Add Toffoli test

* Add CSWAP test

* Update qubit number restriction

* Remove params from 3q gates:

* Apply suggestions from code review

* Make Rot tests clearer

* Add vectorize() function

* Update docstrings (#19)

* Add docstrings for lightning_qubit.cpp

* Update version details

* Update init

* Minor update

* Document lightning_qubit.hpp

* Add operations docstrings

* Update python docs

* Add warning

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>

* Support more gates (#15)

* Partially add multiple qubits

* Nearly working

* Add argsort again

* multiqubit version

* Add CRot

* Add CRZ

* Add CRY

* Add target gates

* Add CRX

* Add CRX

* Add phaseshift

* Add CZ

* add toffoli

* Decide not to refactor

* Merge better

* Add SWAP

* Add CSWAP

* Don't support MultiRZ

* Don't support arbitrary unitaries

* Update gitignore

* Tests not working

* Refactor

* Support multiple qubit numbers

* Support multiple qubit numbers

* Update lightning_qubit.py

* Change iterator type

* Add CRX etc tests

* Add Rot to tests

* Remove extra file

* Remove extras

* Merge properly

* Update

* Add PhaseShift test

* Fix bug

* Remove O3 flag

* Add Toffoli test

* Add CSWAP test

* Update qubit number restriction

* Remove params from 3q gates:

* Apply suggestions from code review

* Make Rot tests clearer

* Add vectorize() function

* Apply suggestions from code review

Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
* Adding list of supported observables (no Hermitian)

* Adding pythontests job

* Renaming jobs

* Installing PennyLane

* Adjusting checking out multiple repos

* Correcting path

* Installing flaky

* Checkout skip_unsupported_ops_tests

* Adding the -rsx option to output messages for skipped test cases

* Update .github/workflows/build.yml

Co-authored-by: Josh Izaac <josh146@gmail.com>

* Remove python version miniconda

* Removing shots spec for analytic False too

* Get PL master and use the --skip-ops option

Co-authored-by: Josh Izaac <josh146@gmail.com>
* Partially add multiple qubits

* Nearly working

* Add argsort again

* multiqubit version

* Add CRot

* Add CRZ

* Add CRY

* Add target gates

* Add CRX

* Add CRX

* Add phaseshift

* Add CZ

* add toffoli

* Decide not to refactor

* Merge better

* Add SWAP

* Add CSWAP

* Don't support MultiRZ

* Don't support arbitrary unitaries

* Update gitignore

* Tests not working

* Refactor

* Support multiple qubit numbers

* Support multiple qubit numbers

* Update lightning_qubit.py

* Change iterator type

* Add CRX etc tests

* Add Rot to tests

* Remove extra file

* Remove extras

* Merge properly

* Update

* Add PhaseShift test

* Fix bug

* Remove O3 flag

* Add Toffoli test

* Add CSWAP test

* Update qubit number restriction

* Remove params from 3q gates:

* Apply suggestions from code review

* Passing the states and other pars by const ref

* Making calc_perm to be an in-place function (by using pass by reference)

* Make Rot tests clearer

* Add vectorize() function

* Reverting to using a tensor_contracted helper variable

* Removing the operation parameter when calling contract_3q_op

* Update 1q version

* Update docstrings (#19)

* Add docstrings for lightning_qubit.cpp

* Update version details

* Update init

* Minor update

* Document lightning_qubit.hpp

* Add operations docstrings

* Update python docs

* Add warning

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>

* Support more gates (#15)

* Partially add multiple qubits

* Nearly working

* Add argsort again

* multiqubit version

* Add CRot

* Add CRZ

* Add CRY

* Add target gates

* Add CRX

* Add CRX

* Add phaseshift

* Add CZ

* add toffoli

* Decide not to refactor

* Merge better

* Add SWAP

* Add CSWAP

* Don't support MultiRZ

* Don't support arbitrary unitaries

* Update gitignore

* Tests not working

* Refactor

* Support multiple qubit numbers

* Support multiple qubit numbers

* Update lightning_qubit.py

* Change iterator type

* Add CRX etc tests

* Add Rot to tests

* Remove extra file

* Remove extras

* Merge properly

* Update

* Add PhaseShift test

* Fix bug

* Remove O3 flag

* Add Toffoli test

* Add CSWAP test

* Update qubit number restriction

* Remove params from 3q gates:

* Apply suggestions from code review

* Make Rot tests clearer

* Add vectorize() function

* Apply suggestions from code review

Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

Co-authored-by: trbromley <brotho02@gmail.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>
* Not working

* Nearly there

* Not there

* Closer

* Remove shuffling

* Remove print

* Remove comments

* Remove extra letter

* Improve docstrings

* Rename function and fix tests

* Update argsort function

* Simplify 1q case

* Update 1q case

* Update 2q case

* Sort out ints in for loops
* Update README

* Update requirements

* Add to changelog

* Remove travis

* More tidying

* Remove unused tests

* Remove qiskit reference

* Use github master branch of PL for now

* Use pl v0.10

* Try this

* Revert to previous requires

* Use old req and setup

* Replace old test

* Lighten setup.py requirements

* Update pl v

* Update requirements
* Configure setup.py to build the C++ source code

* fix

* fix

* fix

* set up CI

* typo

* typo

* more

* more

* more

* more

* more

* more

* more

* more

* more

* more

* more

* more

* more

* more

* more

* please work

* fix

* fix

* fix

* Try mac wheels

* fix

* typo

* typo

* fix

* fix

* fix

* fix

* fix

* try something new

* try something new

* try something new

* please work

* please work

* please work

* please work

* please work

* please work

* try again

* try again

* try again

* try again

* try again

* try again

* FINI

* remove win32 builds

* tweak

* Remove manual make file

* Update setup.py

Co-authored-by: Josh Izaac <josh146@gmail.com>

Co-authored-by: trbromley <brotho02@gmail.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
* Update README

* Update requirements

* Add to changelog

* Remove travis

* More tidying

* Remove unused tests

* Remove qiskit reference

* Use github master branch of PL for now

* Use pl v0.10

* Try this

* Revert to previous requires

* Use old req and setup

* Replace old test

* Lighten setup.py requirements

* Update pl v

* Update requirements

* Clearing up the test file for the apply of lightning.qubit; update cpp

* Remove unused parts of conftest.py

* Add step for unit tests

* Update path

* Adding pre-def matrices, tol

* Adding tests for expval

* Adding tests for var

* Use internal device object in fixtures

* Use internal device object in fixtures more

* Update pennylane_lightning/src/tests/lightning_unittest.cpp

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update module docstring

* Import U from conftest

* Copyrights and test file docstrings

* Update pennylane_lightning/tests/test_apply.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Add Toffoli test

* Remove generate_samples calls

* Using device fixture

Co-authored-by: trbromley <brotho02@gmail.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
* Update README

* Update requirements

* Add to changelog

* Remove travis

* More tidying

* Remove unused tests

* Remove qiskit reference

* Use github master branch of PL for now

* Use pl v0.10

* Try this

* Revert to previous requires

* Use old req and setup

* Replace old test

* Lighten setup.py requirements

* Update pl v

* Update requirements

* Clearing up the test file for the apply of lightning.qubit; update cpp

* Remove unused parts of conftest.py

* Add step for unit tests

* Update path

* Adding pre-def matrices, tol

* Adding tests for expval

* Adding tests for var

* Draft Makefile

* Omit tests folder

* Adding the test-cpp option

* Tweaking dirs

* Remove PLUGIN_TESTRUNNER var

* tweak test args

* Adding google test dir path

* Fixing test-cpp

* Removing generating samples from var tests too

Co-authored-by: trbromley <brotho02@gmail.com>
* Add workflow dispatch event triggering

* Add workflow dispatch event triggering (wheels)

* Remove newline

* Revert to trigger workflows for Testing on pull request
* Add pytest and pytest-cov to requirements

* Add 1q test

* Add 2q circuit

* Add 4 qubit test

* Add n-qubit circuit

* Tidy

* Apply black

* Update based on change in PL

* Update based on changes in PL

* Update due to wires refactor

* Quick fix
* Add to README

* Mention C++ tests
* First attempt

* Update conf

* Update conf

* Add check in setup.py to not install if USING_RTD environment variable is true

* Add an RTD setup file

* Change environment variable name

* Add install location

* Update install location

* Update install location

* Update

* Add install

* Update setup.py

* Update requirements
* Have new path in workflow file

* Updating remaining occurrences
* Use Windows optimization compiler flag

* Further modifying compiler and linker options

* Specifying C++14 option

* Remove C++ version pinning

* Sepcify C++14 version comp option

* Specifying the -std:c++11 option and turning to using the - character

* Building the wheel on push and on pull request

* Correct event trigger

Co-authored-by: trbromley <brotho02@gmail.com>
* Tidy up

* Extra update

* Update setup.py

Co-authored-by: antalszava <antalszava@gmail.com>

* Apply suggestion

Co-authored-by: antalszava <antalszava@gmail.com>
* Add black check

* Run black

* Add codecov report generation

* Add codecov report generation

* Update location

* Update location

* Update for correct path

* Increase coverage

* Isort

* Add shields

* Sort out code quality

* Update build action

* Apply suggestions from code review

Co-authored-by: antalszava <antalszava@gmail.com>

* Use pytest mock

Co-authored-by: antalszava <antalszava@gmail.com>
* Incrementing the version number & requirement number

* fixes

* fix

* fix

Co-authored-by: Josh Izaac <josh146@gmail.com>
* Turning to using the GitLab files for Eigen

* Fixes
@AmintorDusko
Copy link
Contributor

AmintorDusko commented Sep 22, 2022

Hi @multiphaseCFD, nice work so far!
Can we have unit tests for the new methods?
I wonder why in some of your benchmarks the methods executed with float precision are taking longer than their double precision counterparts. Do you have any insight about that?

@multiphaseCFD
Copy link
Member Author

Hi @multiphaseCFD, nice work so far! Can we have unit tests for the new methods? I wonder why in some of your benchmarks the methods executed with float precision are taking longer than their double precision counterparts. Do you have any insight about that?

Thanks @AmintorDusko ! In fact, thanks to @chaeyeunpark 's nice implementation, the newly implemented gates are implicitly tested by the existing testing kernels here. Do you know how to pass-by this errors according to the current situation?
Good question for why double is faster. I believe it's related to hardware architecture. According to tests on Mac M1, fp32 computation is faster than fp64 computation as shown below.

CSWAP
DoubleExcitation
DoubleExcitationMinus
DoubleExcitationPlus
Toffoli

@AmintorDusko
Copy link
Contributor

Hi @multiphaseCFD, nice work so far! Can we have unit tests for the new methods? I wonder why in some of your benchmarks the methods executed with float precision are taking longer than their double precision counterparts. Do you have any insight about that?

Thanks @AmintorDusko ! In fact, thanks to @chaeyeunpark 's nice implementation, the newly implemented gates are implicitly tested by the existing testing kernels here. Do you know how to pass-by this errors according to the current situation? Good question for why double is faster. I believe it's related to hardware architecture. According to tests on Mac M1, fp32 computation is faster than fp64 computation as shown below.

CSWAP DoubleExcitation DoubleExcitationMinus DoubleExcitationPlus Toffoli

Thank you, @multiphaseCFD, for addressing my questions.

Copy link
Contributor

@chaeyeunpark chaeyeunpark left a comment

Choose a reason for hiding this comment

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

Thanks @multiphaseCFD for amazing work! I haven't expected to have LM implementations of four qubit gates (are they in Kokkos?). Let's see the coverage report after merging #359. If it looks fine, I will approve this.

@chaeyeunpark
Copy link
Contributor

I also note that we can let StateVector classes use these kernels by giving information in https://github.com/PennyLaneAI/pennylane-lightning/blob/master/pennylane_lightning/src/simulator/AssignKernelMap_Default.cpp. We may have that as a separate PR.

@multiphaseCFD
Copy link
Member Author

Thanks @multiphaseCFD for amazing work! I haven't expected to have LM implementations of four qubit gates (are they in Kokkos?). Let's see the coverage report after merging #359. If it looks fine, I will approve this.

Thanks @chaeyeunpark ! Yes, I implemented them in L-Kokkos.

Copy link
Contributor

@chaeyeunpark chaeyeunpark left a comment

Choose a reason for hiding this comment

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

Thanks @multiphaseCFD ! One but non-essential comment.

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Just a quick one from me

@@ -66,6 +66,54 @@ class GateImplementationsLM : public PauliGenerator<GateImplementationsLM> {
return {parity_high, parity_middle, parity_low};
}

static auto revWireParity(size_t rev_wire0, size_t rev_wire1,
Copy link
Member

Choose a reason for hiding this comment

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

We can probably define this and the next one as inline. Also, worth a try to make it constexpr too, (need to check if fillTrailingOnes and fillLeadingOnes are compatible).

Also, since we know the return type, it would be good to add it as an arrow type: -> std::array<size_t,parity_size>

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure!

@mlxd
Copy link
Member

mlxd commented Sep 26, 2022

@multiphaseCFD Can we not just reopen this PR and keep everything here? I'd rather avoid splitting the reviews and details over 2 separate PR processes.

@multiphaseCFD multiphaseCFD added WIP Work-in-progress issue. Not for general fixing. and removed WIP Work-in-progress issue. Not for general fixing. labels Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet