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

Create a base LKokkos new device API backend. #810

Merged
merged 113 commits into from
Sep 4, 2024

Conversation

LuisAlfredoNu
Copy link
Contributor

@LuisAlfredoNu LuisAlfredoNu commented Jul 22, 2024

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:
Following the design of Lightning Qubit uses the New Device API end to end, we can now migrate lightning.kokkos device to the new device API.

Description of the Change:
Migration of Lightning Kokkos to integrate the New device API. The list of main features changes are

  • Add the state_vector, measurements, and adjoint-jacobian classes for lightning.kokkos
  • Add the simulate, jacobian, simulate_and_jacobian, vjp, and simulate_and_vjp methods to lighting.kokkos
  • Update unit/integration tests for the new device API to work with lightning.kokkos
  • Check the full support for sampling in full parity with Lightning Qubit
  • Replace the old device API for Lightning Kokkos.

Benefits:
Full integration of Lightning kokkos with the new device API.

Possible Drawbacks:

Related GitHub Issues:

[sc-59207], [sc-68825]

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

Attention: Patch coverage is 26.73267% with 444 lines in your changes missing coverage. Please review.

Project coverage is 74.67%. Comparing base (cc2942e) to head (5561277).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...ane_lightning/lightning_kokkos/lightning_kokkos.py 31.57% 130 Missing ⚠️
...nylane_lightning/lightning_kokkos/_measurements.py 20.38% 125 Missing ⚠️
...nylane_lightning/lightning_kokkos/_state_vector.py 28.26% 99 Missing ⚠️
...ne_lightning/lightning_kokkos/_adjoint_jacobian.py 25.00% 90 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (cc2942e) and HEAD (5561277). Click for more details.

HEAD has 32 uploads less than BASE
Flag BASE (cc2942e) HEAD (5561277)
40 8
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #810       +/-   ##
===========================================
- Coverage   97.84%   74.67%   -23.17%     
===========================================
  Files         117       20       -97     
  Lines       18658     2330    -16328     
===========================================
- Hits        18255     1740    -16515     
- Misses        403      590      +187     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LuisAlfredoNu LuisAlfredoNu marked this pull request as ready for review July 22, 2024 22:21
@LuisAlfredoNu LuisAlfredoNu added the ci:build_wheels Activate wheel building. label Jul 23, 2024
@multiphaseCFD
Copy link
Member

Thanks @LuisAlfredoNu ! Would you like to add a new entry to the .github/CHANGELOG.md file, summarizing the
change, and including a link back to the PR?

@LuisAlfredoNu LuisAlfredoNu marked this pull request as draft July 23, 2024 13:18
@LuisAlfredoNu LuisAlfredoNu marked this pull request as ready for review July 24, 2024 15:07
@LuisAlfredoNu LuisAlfredoNu requested review from multiphaseCFD and a team July 24, 2024 15:08
@vincentmr vincentmr self-requested a review July 24, 2024 18:22
@vincentmr vincentmr added the do not merge Do not merge PR until this label is removed label Jul 24, 2024
Copy link
Contributor

@vincentmr vincentmr left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @LuisAlfredoNu . Not much to complain about at the moment. I'm adding the label "do not merge" to the PR since the description says this will be merged last. But feel free to add/remove whatever label you would like.

pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

I made some suggestions. I think it is better if you really keep the implementation here minimal. I think you left a lot of code that will probably need to be changed. Please, comment out all code that is not strictly necessary, and pass all tests.

pennylane_lightning/lightning_kokkos/_state_vector.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/_state_vector.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/_state_vector.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved
LuisAlfredoNu and others added 2 commits July 24, 2024 15:26
Co-authored-by: Amintor Dusko <[email protected]>
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Super nice job! Thank you for that, @LuisAlfredoNu!

.github/workflows/tests_lkcpu_python.yml Outdated Show resolved Hide resolved
tests/lightning_qubit/test_jacobian_method.py Show resolved Hide resolved
Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

Great work! 🥳 Feel free to merge it after #884.

.github/CHANGELOG.md Outdated Show resolved Hide resolved
@LuisAlfredoNu LuisAlfredoNu removed the do not merge Do not merge PR until this label is removed label Sep 4, 2024
@LuisAlfredoNu LuisAlfredoNu merged commit 492706a into master Sep 4, 2024
101 of 103 checks passed
@LuisAlfredoNu LuisAlfredoNu deleted the kokkosNewAPI_backend branch September 4, 2024 14:46
@LuisAlfredoNu LuisAlfredoNu restored the kokkosNewAPI_backend branch September 4, 2024 15:46
@LuisAlfredoNu LuisAlfredoNu deleted the kokkosNewAPI_backend branch September 4, 2024 15:49
multiphaseCFD pushed a commit that referenced this pull request Sep 5, 2024
Please complete the following checklist when submitting a PR:

- [X] 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`](../tests) directory!

- [X] 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`.

- [X] Ensure that the test suite passes, by running `make test`.

- [X] Add a new entry to the `.github/CHANGELOG.md` file, summarizing
the
      change, and including a link back to the PR.

- [X] 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:**
Following the design of *Lightning Qubit* uses the New Device API end to
end, we can now migrate `lightning.kokkos` device to the new device API.

**Description of the Change:**
Migration of Lightning Kokkos to integrate the New device API. The list
of main features changes are
- Add the `state_vector`, `measurements`, and `adjoint-jacobian` classes
for `lightning.kokkos`
- Add the `simulate`, `jacobian`, `simulate_and_jacobian`, `vjp`, and
`simulate_and_vjp` `methods to lighting.kokkos`
- Update unit/integration tests for the new device API to work with
`lightning.kokkos`
- Check the full support for sampling in full parity with Lightning
Qubit
- Replace the old device API for Lightning Kokkos.

**Benefits:**
Full integration of **Lightning kokkos** with the new device API.

**Possible Drawbacks:**

**Related GitHub Issues:**

[sc-59207], [sc-68825]

---------

Co-authored-by: ringo-but-quantum <[email protected]>
Co-authored-by: Amintor Dusko <[email protected]>
Co-authored-by: Vincent Michaud-Rioux <[email protected]>
Co-authored-by: Ali Asadi <[email protected]>
multiphaseCFD pushed a commit that referenced this pull request Sep 8, 2024
### Before submitting

Please complete the following checklist when submitting a PR:

- [X] 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`](../tests) directory!

- [X] 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`.

- [X] Ensure that the test suite passes, by running `make test`.

- [X] Add a new entry to the `.github/CHANGELOG.md` file, summarizing
the
      change, and including a link back to the PR.

- [X] 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:**
Following the design of *Lightning Qubit* uses the New Device API end to
end, we can now migrate `lightning.kokkos` device to the new device API.

**Description of the Change:**
Migration of Lightning Kokkos to integrate the New device API. The list
of main features changes are
- Add the `state_vector`, `measurements`, and `adjoint-jacobian` classes
for `lightning.kokkos`
- Add the `simulate`, `jacobian`, `simulate_and_jacobian`, `vjp`, and
`simulate_and_vjp` `methods to lighting.kokkos`
- Update unit/integration tests for the new device API to work with
`lightning.kokkos`
- Check the full support for sampling in full parity with Lightning
Qubit
- Replace the old device API for Lightning Kokkos.

**Benefits:**
Full integration of **Lightning kokkos** with the new device API. 

**Possible Drawbacks:**

**Related GitHub Issues:**

[sc-59207], [sc-68825]

---------

Co-authored-by: ringo-but-quantum <[email protected]>
Co-authored-by: Amintor Dusko <[email protected]>
Co-authored-by: Vincent Michaud-Rioux <[email protected]>
Co-authored-by: Ali Asadi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:build_wheels Activate wheel building.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants