catalyst icon indicating copy to clipboard operation
catalyst copied to clipboard

[WIP] Add Python-native logging to Catalyst frontend

Open mlxd opened this issue 1 year ago • 2 comments

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, and Runtime 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 folow the same design as PennyLane to support logging with the same unified configuration, allowing ease of examining of function entry points at the DEBUG level, and a custom TRACE method for expanding functions passed as arguments.

Description of the Change: Adds preliminary support to a variety of Catalyst frontend entry-points, allowing the 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: Current implementation can likely be replaced with decorator-level additions to reduce additional code added.

Related GitHub Issues: https://github.com/PennyLaneAI/pennylane/pull/5528

mlxd avatar Apr 16 '24 19:04 mlxd

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.07%. Comparing base (b7ad7db) to head (18c20fd). Report is 184 commits behind head on main.

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              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 16 '24 21:04 codecov[bot]

[sc-61592]

mlxd avatar Apr 17 '24 21:04 mlxd

Looks good to me 💯

Is this a new requirement now for all new functions, classes, and modules going forward?

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.

mlxd avatar Jun 03 '24 13:06 mlxd