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

Catalyst support for qml.CosineWindow #1166

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

willjmax
Copy link

@willjmax willjmax commented Oct 1, 2024

Compiling a circuit with QJIT that begins with a CosineWindow state preparation results in a compiler error. This is because Catalyst has optimizations for when the first operation is a state preparation, but these optimizations are not supported for CosineWindow. A workaround is to only perform the state preparation optimizations on the StatePrep and BasisState classes, rather than the more general StatePrepBase class.

This workaround requires additional changes to PennyLane (PR #6318) and PennyLane lightning (PR #929).

…l add support to qml.qml.CosineWindow when it's called as the first op in tapes
@willjmax willjmax changed the title Catalyst support for qml.CosineWindow Catalyst support for qml.CosineWindow Oct 1, 2024
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.

Thanks for the fix @willjmax! 🎉 Just some formatting changes below, but we should also make sure to add a test to confirm the template is working (e.g. in catalyst/frontend/test/pytest/test_templates.py).

frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
.dep-versions Outdated Show resolved Hide resolved
frontend/catalyst/device/verification.py Outdated Show resolved Hide resolved
.dep-versions Outdated
Comment on lines 13 to 16
# For a custom LQ/LK version, update the package version here and at
# 'doc/requirements.txt'. Also, update the 'LIGHTNING_GIT_TAG' at
# 'runtime/Makefile' and at all GitHub workflows, using the exact
# commit hash corresponding to the merged PR that implements the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make sure to also update the LIGHTNING_GIT_TAG variable as described in this comment :)

Copy link
Author

Choose a reason for hiding this comment

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

Could you provide some more details on this? This is my first Catalyst contribution and I'm not sure I understand the comment 😕

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, the process to update the dependency requires setting the version in a few places at the moment (this will become easier some time this quarter), the .dep-versions file you already noticed, and

There are two kinds of dependencies, "version strings" which can be used by pip when installing packages, and git refs which can be used by git when cloning repositories. For lightning because we currently build the lightning qubit device in the runtime, we have to make sure the git commit we use for this is the same as the lightning python package that will be installed by pip. To match the two take a particular PR that you want to depend on, say PennyLaneAI/pennylane-lightning#929. In the PR we see the package version string assigned to it:
PennyLaneAI/pennylane-lightning@02140db
Once the PR is merged, you will be able to see the merge commit on main and get its hash. Use this as the LIGHTNING_GIT_TAG.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For pennylane the idea is the same, although you don't need to worry about the git tag. Wait until the pennylane PR is merged, then get the package version that contains the PR you want. PennyLane updates its version and pushes a development package to pypi every night (contrary to lightning which does it every PR).
If you want the released dev package quicker, you can also trigger the action manually.

@dime10
Copy link
Collaborator

dime10 commented Oct 2, 2024

Lastly, you'll need a changelog entry, you can put your change in the improvements section:

<h3>Improvements</h3>

Copy link

codecov bot commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.93%. Comparing base (d4236ac) to head (c3afe75).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1166   +/-   ##
=======================================
  Coverage   97.93%   97.93%           
=======================================
  Files          77       77           
  Lines       10921    10921           
  Branches      971      971           
=======================================
  Hits        10695    10695           
  Misses        179      179           
  Partials       47       47           

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

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.

3 participants