-
Notifications
You must be signed in to change notification settings - Fork 603
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
Allow specifying error type to be raised by decompose #5669
Conversation
Hello. You may have forgotten to update the changelog!
|
1 similar comment
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5669 +/- ##
==========================================
- Coverage 99.69% 99.68% -0.02%
==========================================
Files 412 412
Lines 38663 38367 -296
==========================================
- Hits 38545 38245 -300
- Misses 118 122 +4 ☔ View full report in Codecov by Sentry. |
[sc-61450] |
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.
The change looks good to me, thanks @lillian542! 🎉
Would be good to update the Args:
section as the comment details.
Also, do we want a basic test that asserts the error
kwarg being used correctly?
And a changelog entry? 🤔
Thanks @lillian542!
Are you able to speak more towards that motivation? I guess this is needed for Catalyst integration, but I'm just curious to understand more. |
@dwierichs - why yes. Yes we do want those things 🙈 |
@trbromley - this function was originally used only in preprocessing for standard devices (which we want to raise a As of this week, it's also used in transforms, where no device is present, and it would be better to raise the original So the motivation is to keep error types consistent with what process is failing, even though at the end of the day they all come down to a An alternative would be to always use |
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.
Thanks @lillian542
Just 1.5 comments regarding the docstring.
Co-authored-by: David Wierichs <david.wierichs@xanadu.ai>
Context:
As the
decompose
transform is used in various contexts, it would be nice to specify the error type that should be raised if its not possible to get a suitable decomposition for one of the operators (i.e. devices raiseDeviceError
, transforms and gradients raiseDecompositionUndefinedError
, catalyst raisesCompileError
)Description of the Change:
An error kwarg is added to
decompose
and its helper function_operator_decomposition_gen
so that the error type can be specified. It continues to default toDeviceError
Benefits:
We don't have to duplicate many lines of code to get an identical function with a different error type
Possible Drawbacks:
More kwargs
Related GitHub Issues:
N/A
Related Shortcut Stories:
[sc-61450]