Add tracing instrumentation to nuopc driver #162
Add tracing instrumentation to nuopc driver #162jiandewang merged 13 commits intoNOAA-EMC:dev/emcfrom
Conversation
|
@DusanJovic-NOAA is the module or a stub for the: If not:
|
No.
This tracing is specific to UFS, so when this repo is built as a subcomponent of ufs-weather-model users have an option to set
What would that dummy stub do? As long as |
|
@DusanJovic-NOAA Passing your response to others with whom we co-share this coupler (e.g, NCAR) and see what they have to say. Cc: @alperaltuntas |
|
One example of the ways stubs are used in the component caps is in CICE. There is a file called |
I understand that the code will not have |
|
Isn't our build-system isolated from anyone else's because we have a MOM6 (or CICE, or CMEPS) interface w/ our own CMakeList etc? |
Yes, and since we (ufs-weather-model) control it, our build system is not a problem. But if we add the stub module, everybody else will need to update their build system to compile that new stub file. And until they update, their build will be broken. While by using Anyway, if |
|
Thanks Dusan. Lets see what feedback we get from NCAR side. I personally have an aversion to bunches of ifdefs, but if other users of the cap don't mind, then that is fine. I was initially responding to the comment about whether this could be a stub. Regarding breaking the build of other systems, in both WW3 and CICE we have existing wrapper routines which could be utilized. Unfortunately not in CMEPS. |
|
I'm not a fan of if def (compiler directives) either! |
|
from NCAR: |
|
Would it be acceptable to replace the stub module with the wrapper module (like we did in CMEPS, for example) and update all current call statements to call the corresponding wrapper routine. That wrapper module will still have few ifdefs, but they will be located in that single module. The problem with having this stub module defined in this repo is that there could be only one of those, and if for some reason any other component defines it, there is a linker error due to multiple definitions. |
|
@sanAkel can you review this PR ? I think the associated UWM PR is pending on this submodule PR |
sanAkel
left a comment
There was a problem hiding this comment.
Thanks for adding the ufs_trace_wrapper module. Please see requested change(s).
@jiandewang ✔️ |
@DusanJovic-NOAA can you modify the above as you are not using "DUSE" any more |
UFS_TRACING preprocessor macro is still used in mom_ufs_trace_wrapper.F90 |
|
Testing on WM PR 2884 has completed successfully. This PR can be merged. |
|
MOM6 merged, hash # e5a7888 |
This PR adds calls to ufs tracing routines that will create a trace file which can then be visualized, which is found to be useful in identifying various performance issues.
See ufs-weather-model issue ufs-community/ufs-weather-model#2883
Added calls are not used by default, unless build option (-DUFS_TRACING=ON) is specified when building the ufs-weather-model.
No changes in answers are expected.