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 Lightning device TOML files to the new schema #988

Merged
merged 73 commits into from
Nov 22, 2024
Merged

Conversation

astralcai
Copy link
Contributor

@astralcai astralcai commented Nov 7, 2024

Context:

This PR is part of the new device capabilities initiative to improve feature parity across the ecosystem. See this ADR for more context. A new TOML schema has been defined, and the relevant module implemented in PennyLane:

As well as updates made to Catalyst:

Description of the Change:

  • Updates lightning_qubit.toml, lightning_kokkos.toml, lightning_gpu.toml to the new schema
  • Removes _operations and _observables from LightningQubit, LightningKokkos, and LightningGPU as they are now available via Device.capabilities that is loaded from the TOML file.

Benefits:

A step towards feature parity across the ecosystem.

Possible Drawbacks:

  • Per discussions when the ADR was developed, operator.gates.decomp and operator.gates.matrix are removed, and the TOML file no longer prescribes to the framework how an operator should be handled. To ensure consistency of behaviour, this information is temporarily moved to a _to_matrix_ops class property to be used by Catalyst, until we have better support for customizable multi-pathway decompositions.

Related GitHub Issues:

[sc-71729]
[sc-77214]

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 62.85714% with 13 lines in your changes missing coverage. Please review.

Project coverage is 92.14%. Comparing base (9fc9633) to head (71f4109).
Report is 51 commits behind head on master.

Files with missing lines Patch % Lines
...ylane_lightning/lightning_qubit/lightning_qubit.py 58.33% 5 Missing ⚠️
pennylane_lightning/lightning_gpu/lightning_gpu.py 63.63% 4 Missing ⚠️
...ane_lightning/lightning_kokkos/lightning_kokkos.py 63.63% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (9fc9633) and HEAD (71f4109). Click for more details.

HEAD has 50 uploads less than BASE
Flag BASE (9fc9633) HEAD (71f4109)
58 8
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #988      +/-   ##
==========================================
- Coverage   97.72%   92.14%   -5.59%     
==========================================
  Files         228      104     -124     
  Lines       36378    16267   -20111     
==========================================
- Hits        35550    14989   -20561     
- Misses        828     1278     +450     

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

@astralcai astralcai added the do not merge Do not merge PR until this label is removed label Nov 11, 2024
@astralcai astralcai marked this pull request as ready for review November 12, 2024 16:35
Base automatically changed from fix_adjops_jacobian to master November 18, 2024 15:32
AmintorDusko
AmintorDusko previously approved these changes Nov 19, 2024
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.

Looks good! No blocker from me!

@AmintorDusko AmintorDusko dismissed their stale review November 19, 2024 15:59

Docs are failing.

@astralcai
Copy link
Contributor Author

@AmintorDusko The doc build is failing because it's building with latest PL, which still doesn't contain the changes that this PR depends on. Once we merge the PR into PL, it will unblock the doc build for Lightning.

@AmintorDusko
Copy link
Contributor

@AmintorDusko The doc build is failing because it's building with latest PL, which still doesn't contain the changes that this PR depends on. Once we merge the PR into PL, it will unblock the doc build for Lightning.

Ohh. Thank you for clarifying, Good job here!

@maliasadi maliasadi added urgent Mark a pull request as high priority ci:use-gpu-runner Enable usage of GPU runner for this Pull Request ci:build_wheels Activate wheel building. labels Nov 21, 2024
@astralcai astralcai removed the do not merge Do not merge PR until this label is removed label Nov 21, 2024
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.

Thanks @astralcai! Feel free to merge this after CIs are completed 🏖️

@astralcai astralcai merged commit 45a67d4 into master Nov 22, 2024
94 of 96 checks passed
@astralcai astralcai deleted the toml-updates branch November 22, 2024 17:56
astralcai added a commit to PennyLaneAI/catalyst that referenced this pull request Nov 25, 2024
**Context:**

This PR is part of the new device capabilities initiative to improve
feature parity between PennyLane and Catalyst. See [this
ADR](PennyLaneAI/adrs#78) for more context. A
new TOML schema has been defined, and the relevant module implemented in
PennyLane:
- PennyLaneAI/pennylane#6407
- PennyLaneAI/pennylane#6433.

**Description of the Change:**

- Removes the `toml` module and replaces all usages with imports from
PennyLane
- Update usages of `DeviceCapabilities` to match the new definition in
PennyLaneAI/pennylane#6407
- Removes use of about-to-be-deprecated return_type of MPs
- Updates all TOML files to use the new schema.
- A PR opened in Lightning
PennyLaneAI/pennylane-lightning#988 that updates
the TOML file of the Lightning devices to the new schema.

**Benefits:**

A step towards feature parity between PennyLane and Catalyst.

**Possible Drawbacks:**

- Per discussions when the ADR was developed, `operator.gates.decomp`
and `operator.gates.matrix` are removed, and the TOML file no longer
prescribes to the framework how an operator should be handled. This will
change the behaviour of decomposition for some circuits.

**Related GitHub Issues:**

[sc-71888]
[sc-71728]
[sc-71730]

---------

Co-authored-by: Romain Moyard <[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. ci:use-gpu-runner Enable usage of GPU runner for this Pull Request urgent Mark a pull request as high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants