compiler: Minor tweaks to enable integrated NCU-based profiling - #2937
compiler: Minor tweaks to enable integrated NCU-based profiling#2937FabioLuporini merged 2 commits into
Conversation
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 40.00000% with 21 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## main #2937 +/- ##
==========================================
- Coverage 83.37% 83.36% -0.02%
==========================================
Files 248 248
Lines 51881 51905 +24
Branches 4479 4479
==========================================
+ Hits 43257 43271 +14
- Misses 7867 7879 +12
+ Partials 757 755 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
| configuration.add('profiling', 'basic', list(profiler_registry), impacts_jit=False) | ||
| def profiling_preprocessor(i): | ||
| if isinstance(i, dict): | ||
| return NcuProfiling(i['ncu']) |
There was a problem hiding this comment.
This could use a try/key error for safety
Sorry, something went wrong.
There was a problem hiding this comment.
not necessary, if you're here inside profiling_preprocessor the presence of ncu (or any other legal value in profiler_registry) has already been ensured
DEVITO_PROFILING=XXX:ForwardFletcherTTI python devitopro/recipes/recipes/run.py <...>
gives
Traceback (most recent call last):
........
File "/venv/lib/python3.10/site-packages/devito/parameters.py", line 51, in wrapper
raise ValueError(f"Illegal configuration parameter ({key}, {value}). "
ValueError: Illegal configuration parameter (profiling, {'XXX': 'ForwardFletcherTTI'}). Accepted: ['basic', 'basic1', 'basic2', 'advanced', 'advanced1', 'advanced2', 'ncu', 'advisor']
Sorry, something went wrong.
| # Allow unquoted strings as `k:v` values. | ||
| processed.append(i) | ||
| values = processed | ||
| except AttributeError: |
There was a problem hiding this comment.
Not sure why you need processed and values if you just do values = processed
Sorry, something went wrong.
There was a problem hiding this comment.
there is an eval above. This is essentially to avoid the user having to supply quotes in the env var , that is, instead of doing DEVITO_PROFILING="ncu:'ForwardFletcherTTI'" they can just do DEVITO_PROFILING="ncu:ForwardFletcherTTI"
Sorry, something went wrong.
There was a problem hiding this comment.
Not sure how it answer my comment. All i'm saying is that you do
processed = [] for ...: processed.extend(...) values = processed
while you can just fill
values = [] for ...: values.extend(...)
and be done
Sorry, something went wrong.
There was a problem hiding this comment.
no, values contains... values, already
Sorry, something went wrong.
Bulk of the code is in PRO