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.

Sign up for GitHub

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

Jump to bottom

Remove Lightning Kokkos plugin #974

Merged
merged 9 commits into from
Aug 8, 2024
Merged

Remove Lightning Kokkos plugin #974

merged 9 commits into from
Aug 8, 2024

Conversation

rauletorresc
Copy link
Contributor

@rauletorresc rauletorresc commented Jul 29, 2024 •
edited
Loading

Context: Lightning Kokkos plugin has been migrated to Lightning. We can remove its code from Catalyst then. But this PR in Lightning must be approved first: PennyLaneAI/pennylane-lightning#837

Description of the Change: Remove plugin from code and from the list of provided plugins.

Benefits: Remove duplicated functionality.

[sc-69450]

@rauletorresc rauletorresc added runtime Pull requests that update the runtime author:build-wheels Run the wheel building workflows on this Pull Request labels Jul 29, 2024
@rauletorresc rauletorresc requested review from maliasadi and a team July 29, 2024 22:15
@rauletorresc rauletorresc self-assigned this Jul 29, 2024
@rauletorresc
Copy link
Contributor Author

rauletorresc commented Jul 29, 2024 •
edited
Loading

Is this approach correct?

@rauletorresc rauletorresc force-pushed the raultorres/remove_kokkos_plugin branch 2 times, most recently from 50fc116 to 6d9457d Compare July 30, 2024 05:38
@dime10
Copy link
Collaborator

dime10 commented Jul 30, 2024

Is this approach correct?

That looks right to me. The next step is updating the lightning-kokkos dependency to a recent dev wheel and checking whether we can run all our tests.

Actually, I'm wondering whether the PR that moved the plugin also ensured the plugin was distributed with the their wheels. I guess we will find out 😅

@paul0403
Copy link
Contributor

Actually, I'm wondering whether the PR that moved the plugin also ensured the plugin was distributed with the their wheels. I guess we will find out 😅

This is the lightning PR:
PennyLaneAI/pennylane-lightning#770

@rauletorresc rauletorresc force-pushed the raultorres/remove_kokkos_plugin branch from e165276 to 2142907 Compare August 2, 2024 00:46
@dime10
Copy link
Collaborator

dime10 commented Aug 2, 2024

Removing the wheel build label until we've finished repairing the actions.

@dime10 dime10 removed the author:build-wheels Run the wheel building workflows on this Pull Request label Aug 2, 2024
@rauletorresc rauletorresc mentioned this pull request Aug 3, 2024
Merged
@rauletorresc rauletorresc marked this pull request as ready for review August 3, 2024 17:40
@codecov Codecov
Copy link

codecov bot commented Aug 3, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.92%. Comparing base (429f107) to head (f090387).

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #974   +/-   ##
=======================================
  Coverage   97.92%   97.92%           
=======================================
  Files          75       75           
  Lines       10548    10548           
  Branches     1216     1216           
=======================================
  Hits        10329    10329           
  Misses        172      172           
  Partials       47       47

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

dime10
dime10 approved these changes Aug 5, 2024
View reviewed changes
Copy link
Collaborator

@dime10 dime10 left a comment

Choose a reason for hiding this comment

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

Love to see it! Let's wait until the lightning PR is merged and also until we can test all wheel configurations, but this has my preliminary approval :)

doc/dev/installation.rst Show resolved Hide resolved
@dime10 dime10 added the reviewer:require-wheels Pull Requests will need wheel building job successful before being merged label Aug 5, 2024
@dime10 dime10 added author:build-wheels Run the wheel building workflows on this Pull Request and removed author:build-wheels Run the wheel building workflows on this Pull Request labels Aug 6, 2024
@dime10
Copy link
Collaborator

dime10 commented Aug 6, 2024

Just waiting on the lightning PR to finish testing these changes 👍

rauletorresc added a commit to PennyLaneAI/pennylane-lightning that referenced this pull request Aug 7, 2024
@rauletorresc @ringo-but-quantum
Implement get_c_interface method for Kokkos plugin (#837)
8f517d2 
**Context:** Catalyst is removing the Kokkos plugin from its code base
(PennyLaneAI/catalyst#974) and is attempting to
use it directly from Lightning. However, the C interface was not
provided via Python.

**Description of the Change:** Implement the C interface provider method
in Python.

**Benefits:** Catalyst can use the Kokkos plugin.

---------

Co-authored-by: ringo-but-quantum <[email protected]>
@rauletorresc rauletorresc force-pushed the raultorres/remove_kokkos_plugin branch from ae4c431 to f333109 Compare August 7, 2024 21:15
@rauletorresc rauletorresc force-pushed the raultorres/remove_kokkos_plugin branch from f333109 to e4faa8b Compare August 7, 2024 21:24
dime10
dime10 reviewed Aug 7, 2024
View reviewed changes
.dep-versions Outdated Show resolved Hide resolved
@dime10 dime10 added the author:build-wheels Run the wheel building workflows on this Pull Request label Aug 7, 2024
@rauletorresc rauletorresc merged commit f82d2ca into main Aug 8, 2024
67 checks passed
@rauletorresc rauletorresc deleted the raultorres/remove_kokkos_plugin branch August 8, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dime10 dime10 dime10 approved these changes

@maliasadi maliasadi Awaiting requested review from maliasadi

Assignees

@rauletorresc rauletorresc

Labels
author:build-wheels Run the wheel building workflows on this Pull Request reviewer:require-wheels Pull Requests will need wheel building job successful before being merged runtime Pull requests that update the runtime
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rauletorresc @dime10 @paul0403 @maliasadi