-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add Python-native logging to Catalyst frontend #660
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #660 +/- ##
==========================================
+ Coverage 98.05% 98.07% +0.02%
==========================================
Files 69 70 +1
Lines 9547 9665 +118
Branches 764 764
==========================================
+ Hits 9361 9479 +118
Misses 151 151
Partials 35 35 ☔ View full report in Codecov by Sentry. |
[sc-61592] |
3c4b869
to
f5fdbb5
Compare
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.
Looks good to me 💯
Is this a new requirement now for all new functions, classes, and modules going forward?
Co-authored-by: David Ittah <dime10@users.noreply.github.com>
Not a hard requirement, but anything that will be part of the public execution API would be recommended to be added. For now, public API endpoints of PennyLane's execution pipeline and the Catalyst frontend are the ones with additions. If it will provide benefit (as in transformations, intermediate modifications, etc) it's likely worth adding. For simple I/O, likely can be ignore. tl;dr discretionary additions are all good. |
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 @mlxd! Could you also update the changelog with a user example for this logging support?
Before submitting
Please complete the following checklist when submitting a PR:
All new functions and code must be clearly commented and documented.
Ensure that code is properly formatted by running
make format
.The latest version of black and
clang-format-14
are used in CI/CD to check formatting.All new features must include a unit test.
Integration and frontend tests should be added to
frontend/test
,Quantum dialect and MLIR tests should be added to
mlir/test
, andRuntime tests should be added to
runtime/tests
.When all the above are checked, delete everything above the dashed
line and fill in the pull request template.
Context: PennyLane's logging support allows developers to explore the execution pipeline end-to-end, while tying into the Python-native logging framework. This has the advantage of being enterprise ready, consumable by a variety of log sinks, and unifies/removes the need for adding
print
statements throughout a code-base, with all control to be handled by the Python language standard libraries. This PR aims to unify Catalyst to follow the same design as PennyLane to support logging with the same unified configuration, allowing ease of examining of function entry points at theDEBUG
level, and a customTRACE
method for expanding functions passed as arguments.Description of the Change: Adds preliminary support to a variety of Catalyst frontend public API entry-points. Any workload using
qml.logging.enable_logging()
allows the function entry data to be streamed to the pre-configured log-handlers from PennyLane.Benefits: More easily allows time-order and standardised data formatting for consumption with complex application execution, debugging, and end-to-end validation of call-points.
Possible Drawbacks:
Related GitHub Issues: PennyLaneAI/pennylane#5528