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

Update unit/integration tests for the new device #840

Merged
merged 75 commits into from
Aug 13, 2024

Conversation

LuisAlfredoNu
Copy link
Contributor

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:
Migrate the lightning.kokkos device to the new device API.

Description of the Change:
Unlock and modify all the unit tests for Lightning Kokkos and pass all the CI.

Benefits:
Integration of LighntingKokkos with the new device API.

Possible Drawbacks:

Related GitHub Issues:

Partial / Freezzed PR ⚠️ ❄️

To make a smooth integration of LightningKokkos with the new device API, we set the branch kokkosNewAPI_backend as the base branch target for this development.

The branch kokkosNewAPI_backend has the mock of all classes and methods necessary for the new API. Also, several tests were disabled with

if device_name == "lightning.kokkos":
    pytest.skip("Kokkos new API in WIP.  Skipping.",allow_module_level=True)

Additionally, the CI testing from PennyLane for LKokkos is temporally disabled through commenting on the following lines in .github/workflows files

: # pl-device-test --device ${DEVICENAME} --skip-ops --shots=20000 $COVERAGE_FLAGS --cov-append
: # pl-device-test --device ${DEVICENAME} --shots=None --skip-ops $COVERAGE_FLAGS --cov-append

However, these tests will unblocked as the implementation progresses.

After all the developments for integrating LightningKokkos with the new API have been completed then the PR will be open to merging to master

[sc-68823]

LuisAlfredoNu and others added 30 commits July 25, 2024 15:31
Co-authored-by: Vincent Michaud-Rioux <[email protected]>
@LuisAlfredoNu LuisAlfredoNu added the ci:build_wheels Activate wheel building. label Aug 7, 2024
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.00%. Comparing base (9a20ee1) to head (d4d3a8f).
Report is 4 commits behind head on kokkosNewAPI_backend.

Files Patch % Lines
...ane_lightning/lightning_kokkos/lightning_kokkos.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           kokkosNewAPI_backend     #840      +/-   ##
========================================================
+ Coverage                 67.08%   73.00%   +5.92%     
========================================================
  Files                        20       20              
  Lines                      2297     2297              
========================================================
+ Hits                       1541     1677     +136     
+ Misses                      756      620     -136     

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

Comment on lines +253 to +254
pl-device-test --device ${DEVICENAME} --skip-ops --shots=20000 $COVERAGE_FLAGS --cov-append
pl-device-test --device ${DEVICENAME} --shots=None --skip-ops $COVERAGE_FLAGS --cov-append
Copy link
Contributor Author

@LuisAlfredoNu LuisAlfredoNu Aug 12, 2024

Choose a reason for hiding this comment

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

All the PL base tests now are uncomment for CI with GitHub Actions

@@ -431,7 +431,7 @@ class LightningKokkos(Device):
# pylint: disable=too-many-instance-attributes

# General device options
_device_options = ("rng", "c_dtype", "batch_obs")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove numpy random generator from the device option. This is only applicable to MCMC

Comment on lines +520 to +521
# It is necessary to set the mcmc default configuration to complete the requirements of ExecuteConfig
mcmc_default = {"mcmc": False, "kernel_name": None, "num_burnin": 0, "rng": None}
Copy link
Contributor Author

@LuisAlfredoNu LuisAlfredoNu Aug 12, 2024

Choose a reason for hiding this comment

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

Setting the mcmc default values in a hardcode way to avoid complaints from PL-base and prevent user modify

Comment on lines -29 to -31
if device_name == "lightning.kokkos":
pytest.skip("Kokkos new API in WIP. Skipping.", allow_module_level=True)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the unit / integration tests now are uncommented to check the functionality of the new device API with Kokkos

@LuisAlfredoNu LuisAlfredoNu marked this pull request as ready for review August 12, 2024 22:44
.github/workflows/tests_lkcuda_python.yml Outdated Show resolved Hide resolved
.github/workflows/tests_lkcuda_python.yml Outdated Show resolved Hide resolved
.github/workflows/tests_lkcuda_python.yml 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.

Thank you for that!

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.

LGTM! 🎉

@LuisAlfredoNu LuisAlfredoNu merged commit fe0954a into kokkosNewAPI_backend Aug 13, 2024
89 of 90 checks passed
@LuisAlfredoNu LuisAlfredoNu deleted the kokkosNewAPI_unit_integration branch August 13, 2024 19:21
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.

3 participants