fix[next]: fix metrics collection when compiling the same program multiple times#2504
Conversation
…ls derived from the same program
There was a problem hiding this comment.
Pull request overview
This pull request refactors the metrics instrumentation system to support multiple compiled program pools derived from the same program definition and backend. The key changes include:
Changes:
- Renamed metrics API functions for clarity:
metrics_context→metrics_context_key,metrics_setter_at_enter→metrics_context_key_at_enter - Introduced pool-specific metrics tracking with a counter mechanism to distinguish between multiple pools with the same (program_name, backend_name) combination
- Refactored hook signatures to pass
program_poolobjects instead of individual compiled programs, enabling better metrics association
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/gt4py/next/instrumentation/metrics.py | Renamed context manager functions for clarity |
| src/gt4py/next/otf/compiled_program.py | Added pool counter and metrics source key generation, updated hook signatures |
| src/gt4py/next/ffront/decorator.py | Updated to use renamed metrics API |
| tests/next_tests/unit_tests/instrumentation_tests/test_metrics.py | Updated test to use renamed API |
| tests/next_tests/integration_tests/feature_tests/instrumentation_tests/test_hooks.py | Updated hooks and tests to match new signatures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
src/gt4py/next/otf/compiled_program.py:130
- The docstring for
compiled_program_call_contextstill documents parameters (compiled_program,root) that are no longer in the function signature, and doesn’t describeprogram_pool. Please update the Args section to match the new signature to avoid confusing hook implementers.
"""
Hook called at the beginning and end of a compiled program call.
Args:
compiled_program: The compiled program being called.
args: The arguments with which the program is called.
kwargs: The keyword arguments with which the program is called.
offset_provider: The offset provider passed to the program.
root: The root of the compiled programs pool this program belongs to, i.e. a tuple of
(program name, backend name).
key: The key of the compiled program in the compiled programs pool.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Only used to provide a more meaningful name to the metrics source key | ||
| _pools_per_root: collections.Counter = collections.Counter() |
There was a problem hiding this comment.
| # Only used to provide a more meaningful name to the metrics source key | |
| _pools_per_root: collections.Counter = collections.Counter() | |
| #: Counter that uniquely identifies a program pool of a program | |
| # When multiple instances of the same program with the same root exist | |
| # (e.g. same backend, but different backend options) each instance has its own | |
| # pool. This counter identifies them and e.g. allows us to give a more meaningful name | |
| # to the metrics source key. | |
| _pools_per_root: collections.Counter = collections.Counter() |
Refactor the instrumentation metrics to support multiple compiled program pools derived from the same program definition and backend.
Changes: