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

Bugfix Catalyst: add GlobalPhase in TOML files #615

Merged
merged 10 commits into from
Feb 22, 2024
Merged

Conversation

vincentmr
Copy link
Contributor

@vincentmr vincentmr commented Feb 16, 2024

…os and L-GPU.

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:
Catalyst tests fail with

frontend/test/pytest/test_transform.py::test_batch_params[lightning.qubit] - catalyst.utils.exceptions.CompileError: Gates in qml.device.operations and specification file do not match

following the introduction of GlobalPhase support (#579).

Description of the Change:
Add GlobalPhase in TOML files.
Add tests that validate the adjoint method for all supported gates.
Add GlobalPhase generators in L-GPU and L-Kokkos.

Benefits:

Possible Drawbacks:

Related GitHub Issues:
Bug fix update Catalyst TOML file [sc-56992]

@vincentmr vincentmr changed the title Add adjoint tests for all gates. Add GlobalPhase generators in L-Kokk… Bugfix Catalyst: add GlobalPhase in TOML files Feb 16, 2024
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (102d848) 91.04% compared to head (fa3be3c) 98.69%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #615      +/-   ##
==========================================
+ Coverage   91.04%   98.69%   +7.65%     
==========================================
  Files          10      169     +159     
  Lines        1686    23934   +22248     
==========================================
+ Hits         1535    23622   +22087     
- Misses        151      312     +161     

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

@vincentmr vincentmr marked this pull request as ready for review February 16, 2024 17:58
@vincentmr vincentmr requested review from erick-xanadu and a team February 16, 2024 17:59
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.

🙌

tests/test_adjoint_jacobian.py Outdated Show resolved Hide resolved
tests/test_adjoint_jacobian.py Outdated Show resolved Hide resolved
@vincentmr vincentmr requested a review from mlxd February 22, 2024 13:59
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.

Thanks @vincentmr

As a follow up, we can track whether the PS recipe needs specific handling for the GlobalPhase input parameter (which shouldn't be trainable).

@vincentmr vincentmr merged commit 9719855 into master Feb 22, 2024
85 checks passed
@vincentmr vincentmr deleted the bugfix/update_toml branch February 22, 2024 15:12
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.

6 participants