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] Add support for native Python if statements via AutoGraph #235

Merged
merged 35 commits into from
Aug 25, 2023

Conversation

dime10
Copy link
Collaborator

@dime10 dime10 commented Aug 9, 2023

Adds imperative style if-statement support to Catalyst.

The feature is based on the autograph module from tensorflow. Besides the if-statement transformation itself, this PR contains all necessary changes to integrate autograph into the Catalyst pipeline.

Tensorflow is assumed to be installed for tests (not added to requirements.txt due to the large package size!), but is otherwise an optional dependency. The user will receive a warning if they try to use it without Tensorflow being installed.

A bug in the cond function is also fixed that didn't raise an error for mismatched return types in a classical tracing context.

[sc-41286]

@dime10 dime10 force-pushed the python_cf branch 2 times, most recently from a011853 to f9d7b51 Compare August 11, 2023 15:16
The verifier function would always exclude the last return type
(typically the quantum register), regardless of wether tracing took
place in a QNode or not.
FileCheck tests are updated to check for code snippets in the autograph
style, rather than the more direct style originally expected from a
custom implementation.
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #235 (5faa3e9) into main (f4c1719) will increase coverage by 0.04%.
Report is 2 commits behind head on main.
The diff coverage is 98.96%.

@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
+ Coverage   99.26%   99.31%   +0.04%     
==========================================
  Files          38       41       +3     
  Lines        6923     7115     +192     
  Branches      343      370      +27     
==========================================
+ Hits         6872     7066     +194     
+ Misses         28       27       -1     
+ Partials       23       22       -1     
Files Changed Coverage Δ
frontend/catalyst/ag_utils.py 94.59% <94.59%> (ø)
frontend/catalyst/__init__.py 94.11% <100.00%> (+0.36%) ⬆️
frontend/catalyst/ag_primitives.py 100.00% <100.00%> (ø)
frontend/catalyst/autograph.py 100.00% <100.00%> (ø)
frontend/catalyst/compilation_pipelines.py 100.00% <100.00%> (ø)
frontend/catalyst/compiler.py 100.00% <100.00%> (ø)
frontend/catalyst/jax_primitives.py 97.04% <100.00%> (+0.02%) ⬆️
frontend/catalyst/pennylane_extensions.py 99.11% <100.00%> (ø)

... and 3 files with indirect coverage changes

Particular emphasis is placed on ensuring that QJIT/QNode decorators
work with autograph in various configurations.
Copy link
Contributor

@pengmai pengmai left a comment

Choose a reason for hiding this comment

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

Incredible work! 🥳 I'm very excited at how nice the UI is. Just a few questions.

frontend/catalyst/ag_primitives.py Show resolved Hide resolved
frontend/test/pytest/test_autograph.py Show resolved Hide resolved
frontend/test/lit/test_autograph.py Show resolved Hide resolved
Copy link
Contributor

@sergei-mironov sergei-mironov left a comment

Choose a reason for hiding this comment

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

Thanks, David! Please see a few technical questions and suggestions from my side.

frontend/catalyst/autograph.py Outdated Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Outdated Show resolved Hide resolved
frontend/catalyst/autograph.py Show resolved Hide resolved
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Nice work @dime10! I mostly just reviewed the docstring

frontend/catalyst/compilation_pipelines.py Outdated Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Outdated Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Outdated Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Outdated Show resolved Hide resolved
frontend/catalyst/compilation_pipelines.py Outdated Show resolved Hide resolved
Public API functions for autograph can always be imported, in particular
the documentation builder can extract their docstrings without a
tensorflow installation.
@dime10 dime10 force-pushed the python_cf branch 2 times, most recently from 614bbf5 to 124a00a Compare August 21, 2023 19:28
@PennyLaneAI PennyLaneAI deleted a comment from github-actions bot Aug 21, 2023
@sergei-mironov sergei-mironov dismissed their stale review August 23, 2023 15:56

Just checking is it possible to remove a change request without explicit approval

@sergei-mironov
Copy link
Contributor

@dime10 please see my comment in the remaining discussion and I'm OK with closing it! The QNode copying problem worries me a little, but more in generic rather than in relation to this particular PR. While checking the sources I found that we already use a similar pattern here. I think I'll ask the PL team what do they think about the copying policies.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Nice work @dime10!

doc/changelog.md Outdated Show resolved Hide resolved
frontend/catalyst/ag_utils.py Show resolved Hide resolved
frontend/catalyst/ag_utils.py Outdated Show resolved Hide resolved
frontend/catalyst/ag_utils.py Outdated Show resolved Hide resolved
frontend/catalyst/ag_utils.py Outdated Show resolved Hide resolved
frontend/catalyst/ag_utils.py Outdated Show resolved Hide resolved
frontend/catalyst/ag_utils.py Show resolved Hide resolved
frontend/catalyst/ag_utils.py Show resolved Hide resolved
frontend/catalyst/ag_utils.py Outdated Show resolved Hide resolved
frontend/catalyst/pennylane_extensions.py Outdated Show resolved Hide resolved
@dime10 dime10 merged commit 82214fe into main Aug 25, 2023
21 of 22 checks passed
@dime10 dime10 deleted the python_cf branch August 25, 2023 19:27
dime10 added a commit that referenced this pull request Sep 21, 2023
Introduces support for Python `for .. in ...:` statement capture as part
of the compiled program.
Similar to PR #235, AutoGraph is used to convert such statements into
the equivalent Catalyst version before tracing occurs. Specifically, the
following constructs are supported via AutoGraph:
- `for elem in iterable:` - These get converted into a `for_loop(0,
len(iterable), 1)` with `elem = iterable[i]` automatically assigned
using the iteration index, assuming `iterable` is convertible to a JAX
array. If this is not the case, the loop is executed as is in Python.
- `for i in range(start, stop, step):` These are converted directly into
their equivalent `for_loop(start, stop, step)`. Contrary to the default
Python `range`, when AutoGraph is enabled `range` can also accept
dynamic tracers as `start`, `stop`, `step` values. If _any_ exception is
raised during the tracing of the `for_loop` body, Catalyst will fall
back to Python with a warning.
- `for i, elem in enemurate(iterable):` - These get converted into
`for_loop(0, len(iterable), 1)` with the iteration index assigned to the
variable chosen by the user (in this case `i`), and `elem =
iterable[i]`. This also assumes that `iterable` is convertible to an
array, and that the loop body traces without exception, otherwise the
loop is executed in Python.

Note that a warning is raised when when a Python fallback is triggered
due to a tracing exception. Python fallbacks caused by the iterable not
being convertible to array are silent.

[sc-41287]
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