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

Add instrumentation to Catalyst's frontend #597

Merged
merged 20 commits into from
Apr 26, 2024
Merged

Add instrumentation to Catalyst's frontend #597

merged 20 commits into from
Apr 26, 2024

Conversation

dime10
Copy link
Contributor

@dime10 dime10 commented Mar 11, 2024

Adds frontend instrumentation to QJIT class methods. To use the instrumentation a session needs to be started:

with instrumentation("identifier", filename="optional_file_path_for_yaml_results", detailed=<True/False>):
    qjit(my_function)(my_args)

Example output (YAML):

2024-03-14 22:32:56.016040:
  name: circuit
  system:
    os: macOS-13.6.4
    arch: arm64
    python: 3.11.6
  results:
    - pre_compilation:
        walltime: 0.003459
        cputime: 0.003
    - capture:
        walltime: 181.768209
        cputime: 181.65
        programsize: 4689
    - generate_ir:
        walltime: 141.458708
        cputime: 141.301
        programsize: 2985
    - compile:
        walltime: 1545.747416
        cputime: 1260.893
        programsize: 11480
    - run:
        walltime: 5.508291
        cputime: 4.384

Example output (console):

[DIAGNOSTICS] Running pre_compilation           walltime: 0.006 ms      cputime: 0.006 ms    
[DIAGNOSTICS] Running capture                   walltime: 4.08 ms       cputime: 4.08 ms        programsize: 55 lines
[DIAGNOSTICS] Running generate_ir               walltime: 8.417 ms      cputime: 8.417 ms       programsize: 59 lines
[DIAGNOSTICS] Running compile                   walltime: 309.765 ms    cputime: 309.765 ms     programsize: 194 lines
[DIAGNOSTICS] Running run                       walltime: 9.098 ms      cputime: 9.098 ms

[sc-62132]

@dime10 dime10 added the frontend Pull requests that update the frontend label Mar 11, 2024
@dime10 dime10 requested a review from maliasadi March 11, 2024 22:44
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/changelog.md on your branch 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.

@dime10 dime10 changed the base branch from main to maa/timer-compiler-runtime March 11, 2024 22:44
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Attention: Patch coverage is 37.95620% with 85 lines in your changes are missing coverage. Please review.

Project coverage is 98.26%. Comparing base (539ff43) to head (009dafa).
Report is 3 commits behind head on main.

Files Patch % Lines
frontend/catalyst/debug/instruments.py 31.45% 84 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #597      +/-   ##
==========================================
- Coverage   99.15%   98.26%   -0.90%     
==========================================
  Files          64       65       +1     
  Lines        9259     9387     +128     
  Branches      719      735      +16     
==========================================
+ Hits         9181     9224      +43     
- Misses         51      135      +84     
- Partials       27       28       +1     

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

@dime10 dime10 marked this pull request as ready for review March 15, 2024 03:38
Base automatically changed from maa/timer-compiler-runtime to main March 20, 2024 17:20
as it is unreliable/implementation defined when, or if,
this method is called.
@dime10
Copy link
Contributor Author

dime10 commented Apr 10, 2024

@maliasadi Could you have a look? I'll add some tests as well to complete the coverage.

Copy link
Member

@maliasadi maliasadi left a comment

Choose a reason for hiding this comment

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

Thanks @dime10! Nothing more from my side 🎉

Don't forget to update the changelog before merge :)

@dime10 dime10 added this to the v0.6.0 milestone Apr 25, 2024
output in console mode. This is particularly important since the
fine-grained output is printed first, so the coarsegrained results are
represented as a "total" underneath.
The runtime did not use the `timer` function wrapper which checks
wether instrumentation is enabled on each call. Instead, a single global
timer instance is used which only checks the enabled status once when
the runtime library is first loaded into Python.

This would mean that the runtime output could not be obtained during any
Python session that already ran atleast one compiled function.
@dime10 dime10 merged commit 9571cba into main Apr 26, 2024
34 of 37 checks passed
@dime10 dime10 deleted the instrumentation branch April 26, 2024 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Pull requests that update the frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants