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

Expand logging configuration support in PennyLane, and also target Catalyst #5528

Merged
merged 72 commits into from
May 30, 2024

Conversation

mlxd
Copy link
Member

@mlxd mlxd commented Apr 16, 2024

Before submitting

Please complete the following checklist when submitting a PR:

  • All new features must include a unit test.
    If you've fixed a bug or added code that should be tested, add a test to the
    test directory!

  • All new functions and code must be clearly commented and documented.
    If you do make documentation changes, make sure that the docs build and
    render correctly by running make docs.

  • Ensure that the test suite passes, by running make test.

  • Add a new entry to the doc/releases/changelog-dev.md file, summarizing the
    change, and including a link back to the PR.

  • The PennyLane source code conforms to
    PEP8 standards.
    We check all of our code against Pylint.
    To lint modified files, simply pip install pylint, and then
    run pylint pennylane/path/to/file.py.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


Context: To better unify PennyLane's logging support, updates to control PennyLane's logging behaviour enabling use with Catalyst allow a unified way to examine execution pipelines throughout the Pythonic layer. This PR expands the configuration of logging, allowing direct editing by a user, as well as adds new module-level log-levels for Catalyst, and expands jax to also enable logging with jaxlib. The above will be useful to define interactions across components in the ecosystem, and aid in building a model of explicit interactions across the packages.

Description of the Change: As above.

Benefits: Expands the logging framework to be more configurable, and extends upcoming support for Catalyst.

Possible Drawbacks:

Related GitHub Issues:

Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

Copy link

codecov bot commented Apr 16, 2024

Codecov Report

Attention: Patch coverage is 74.65753% with 37 lines in your changes missing coverage. Please review.

Project coverage is 99.53%. Comparing base (f5b805b) to head (3f44012).
Report is 265 commits behind head on master.

Files Patch % Lines
pennylane/logging/configuration.py 29.16% 17 Missing ⚠️
pennylane/logging/formatters/formatter.py 50.00% 8 Missing ⚠️
pennylane/logging/decorators.py 80.00% 6 Missing ⚠️
pennylane/logging/utils.py 45.45% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5528      +/-   ##
==========================================
- Coverage   99.67%   99.53%   -0.15%     
==========================================
  Files         416      418       +2     
  Lines       38686    38873     +187     
==========================================
+ Hits        38562    38691     +129     
- Misses        124      182      +58     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @mlxd! Please could you tag the corresponding Shortcut story?

@mlxd mlxd changed the title Expand logging configuration support to Catalyst [WIP] Expand logging configuration support to Catalyst Apr 17, 2024
@mlxd
Copy link
Member Author

mlxd commented Apr 17, 2024

[sc-61591]

@mlxd mlxd changed the title [WIP] Expand logging configuration support to Catalyst [WIP] Expand logging configuration support in PennyLane to also target Catalyst May 8, 2024
### Before submitting

Please complete the following checklist when submitting a PR:

- [ ] All new features must include a unit test.
If you've fixed a bug or added code that should be tested, add a test to
the
      test directory!

- [ ] All new functions and code must be clearly commented and
documented.
If you do make documentation changes, make sure that the docs build and
      render correctly by running `make docs`.

- [ ] Ensure that the test suite passes, by running `make test`.

- [ ] Add a new entry to the `doc/releases/changelog-dev.md` file,
summarizing the
      change, and including a link back to the PR.

- [ ] The PennyLane source code conforms to
      [PEP8 standards](https://www.python.org/dev/peps/pep-0008/).
We check all of our code against [Pylint](https://www.pylint.org/).
      To lint modified files, simply `pip install pylint`, and then
      run `pylint pennylane/path/to/file.py`.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.


------------------------------------------------------------------------------------------------------------

**Context:** This PR expands the logging functionality in PennyLane for
both internal and inter-library (e.g. Lightning, Catalyst) use.

**Description of the Change:** Add support for decorator-defined logging
functions, and updates the internal usage of logging with this.

**Benefits:** Allows more ease-of-use by developers to include logging
with new functionality to the library.

**Possible Drawbacks:** Does not replace custom-defined log-level
messages and trace-level expansion of functions.

**Related GitHub Issues:**
#5528
@mlxd mlxd changed the title [WIP] Expand logging configuration support in PennyLane to also target Catalyst Expand logging configuration support in PennyLane to also target Catalyst May 8, 2024
@mlxd mlxd marked this pull request as ready for review May 8, 2024 20:46
@mlxd mlxd requested a review from a team May 8, 2024 21:42
mlxd and others added 2 commits May 28, 2024 11:28
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
@mlxd mlxd enabled auto-merge (squash) May 28, 2024 15:28
@mlxd
Copy link
Member Author

mlxd commented May 28, 2024

FYI, the CodeCov complaints are due to the newly added untestable source. We ignore these as pytest-runs are negatively impacted in CI runtime if these are hit.

@mlxd mlxd requested review from dime10, trbromley and a team May 28, 2024 16:55
Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @mlxd

pennylane/logging/configuration.py Outdated Show resolved Hide resolved
pennylane/logging/configuration.py Outdated Show resolved Hide resolved
pennylane/logging/configuration.py Outdated Show resolved Hide resolved
pennylane/logging/configuration.py Show resolved Hide resolved
@mlxd mlxd requested a review from trbromley May 29, 2024 21:52
@mlxd mlxd disabled auto-merge May 30, 2024 15:53
@mlxd mlxd merged commit 8adf29a into master May 30, 2024
38 of 40 checks passed
@mlxd mlxd deleted the update/logging_rules_catalyst branch May 30, 2024 15:53
mlxd added a commit to PennyLaneAI/catalyst that referenced this pull request Jun 5, 2024
### Before submitting

Please complete the following checklist when submitting a PR:

- [x] All new functions and code must be clearly commented and
documented.

- [x] 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`](../frontend/test/),
Quantum dialect and MLIR tests should be added to
[`mlir/test`](../mlir/test/), and
Runtime tests should be added to [`runtime/tests`](../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](https://docs.pennylane.ai/en/stable/introduction/logging.html)
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](https://docs.python.org/3/library/logging.html).
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 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 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

---------

Co-authored-by: David Ittah <dime10@users.noreply.github.com>
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.

4 participants