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

[Frontend] Support for transforms #280

Merged
merged 76 commits into from
Nov 10, 2023

Conversation

erick-xanadu
Copy link
Contributor

@erick-xanadu erick-xanadu commented Sep 18, 2023

Context: Allow PL transforms to be traced in Catalyst

Description of the Change: These changes are implemented on top of the tape-transform PR in Catalyst and the development version of PL. These changes apply transforms that are stored on the device into the tape, potentially generating many tapes. Then each tape is traced individually. At last, the classical co-transform is also traced.

TODO:

  • These changes do not include yet the "one function per tape" invariant.
  • Nested transforms

Benefits: Catalyst support for PL transforms.

[sc-41344]

@erick-xanadu erick-xanadu marked this pull request as ready for review September 21, 2023 14:53
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

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

Great PR, very exciting 💯 Some suggestions but nothing blocking. If you pin to the latest PennyLane and the tests pass. I am happy to approve!

frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
Base automatically changed from tape-transform to main September 21, 2023 18:59
@erick-xanadu
Copy link
Contributor Author

@rmoyard since this depends on the main branch of PL and we currently have a undiscovered bug when integrating with the main branch of PL, I'll leave this for now.

@erick-xanadu erick-xanadu force-pushed the eochoa/2023-09-18/transform-support branch from 293ed18 to 0e385e1 Compare October 25, 2023 17:20
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7c00ec4) 99.63% compared to head (597f2a4) 99.63%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #280   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files          44       44           
  Lines        7666     7734   +68     
  Branches      455      467   +12     
=======================================
+ Hits         7638     7706   +68     
  Misses         14       14           
  Partials       14       14           
Files Coverage Δ
frontend/catalyst/jax_tracer.py 99.68% <100.00%> (+0.08%) ⬆️
frontend/catalyst/pennylane_extensions.py 99.64% <100.00%> (+<0.01%) ⬆️

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

Copy link
Contributor

@rmoyard rmoyard 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 @erick-xanadu great job 💯 It would be nice to gather the possible improvements (relax the return type, support composable transforms) with issues and TODO in the code. Otherwise it looks all good!

frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/test/pytest/test_transform.py Outdated Show resolved Hide resolved
@josh146
Copy link
Member

josh146 commented Oct 28, 2023

Nice work @erick-xanadu! Just a few flyby questions/comments

@erick-xanadu
Copy link
Contributor Author

I'm curious if everything still works if Autograph is turned on?

Transforms are restricted to straight-line programs so I don't think autograph would make much sense here.

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.

Great work @erick-xanadu!! 💯

frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Outdated Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/catalyst/jax_tracer.py Show resolved Hide resolved
frontend/test/pytest/test_transform.py Outdated Show resolved Hide resolved
frontend/test/pytest/test_transform.py Outdated Show resolved Hide resolved
frontend/test/pytest/test_transform.py Show resolved Hide resolved
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.

Great 💯

@erick-xanadu erick-xanadu merged commit 20088c8 into main Nov 10, 2023
23 checks passed
@erick-xanadu erick-xanadu deleted the eochoa/2023-09-18/transform-support branch November 10, 2023 20:44
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.

None yet

4 participants