-
Notifications
You must be signed in to change notification settings - Fork 631
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
run_autograph
is now idempotent
#7001
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
run_autograph
is now idempotentrun_autograph
is now idempotent
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7001 +/- ##
========================================
Coverage 99.60% 99.60%
========================================
Files 484 484
Lines 46141 46249 +108
========================================
+ Hits 45957 46065 +108
Misses 184 184 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
Context:
When support for control flow with
autograph
(through theQNode
execution pipeline) was introduced in #6837 a minor bug was discovered where if you try to applyautograph
to an alreadyautograph
'd function, an ambiguous error would show up,There are a few routes to solve this issue,
Description of the Change:
Option (2) was preferred and so if an already converted function is detected (by the presence of the
ag_uncoverted
attribute), we throw anAutoGraphWarning
and just return early. Now we have the behaviour,Benefits: Enables
autograph
specific tests to run with the defaultautograph=True
argument through theQNode
.Possible Drawbacks:
Suppressed the warnings in the
autograph
test folder. This means that a future test that emits this warning could be missed if we don't pay attention to the WAE action that is run weekly.Fixes [sc-83366]