Add UFS tracing instrumentation to nuopc/cmeps driver#1075
Add UFS tracing instrumentation to nuopc/cmeps driver#1075NickSzapiro-NOAA wants to merge 17 commits intoCICE-Consortium:mainfrom
Conversation
…restart_fh") via UFS configuration as for other components Part of ufs-community/ufs-weather-model#2348
|
This is a draft PR as 1) the UFS PR hasn't been merged yet and 2) to ask if the changes are acceptable to the nuopc/cmeps cap Maybe asking @dabail10 @anton-seaice in particular if there is objection to UFS_TRACING ifdef at this level (and any other preferences/criticisms/...). There is an alternate flavor to have the ifdefs at lower layer more like cice_wrapper_mod.F90 (with wrapper like med_ufs_trace_wrapper_mod in NOAA-EMC/CMEPS#151) |
anton-seaice
left a comment
There was a problem hiding this comment.
All ok from me, we might find that useing the same driver code eventually becomes unmanageable, but for now happy to keep trying to keep it all together.
I should have asked more about the tracing work on Tuesday. Is there a summary repo / issue / notebook somewhere ?
| call ESMF_GridCompGet(gcomp, vm=vm,rc=rc) | ||
| if (ChkErr(rc,__LINE__,u_FILE_u)) return | ||
| call ESMF_VMGet(vm, localpet=mype, rc=rc) | ||
| if (ChkErr(rc,__LINE__,u_FILE_u)) return |
There was a problem hiding this comment.
if (ChkErr(rc,__LINE__,u_FILE_u)) return
call ESMF_VMGet(vm, localpet=mype, rc=rc)
if (ChkErr(rc,__LINE__,u_FILE_u)) return
Are these used anywhere?
There was a problem hiding this comment.
to set mype (before my_task and master_task are available in InitializeAdvertise)
dabail10
left a comment
There was a problem hiding this comment.
This generally looks fine, but we already have variables for this sort of thing in CICE.
| if (ChkErr(rc,__LINE__,u_FILE_u)) return | ||
|
|
||
| #ifdef UFS_TRACING | ||
| if (mype == 0) call ufs_trace_init() |
There was a problem hiding this comment.
We should remain consistent with the rest of the code and check:
if (my_task == master_task)
There was a problem hiding this comment.
Yes, I thought the same. But, the tracing starts before my_task and master_task are available in InitializeAdvertise
There was a problem hiding this comment.
Can we call init_grid1 before this phase?
There was a problem hiding this comment.
Maybe not (?) as I think call input_data has to happen before call init_grid1 like in cice_init1 to have all the namelist variables
But, it's more that conflicts with the intent to start the tracing as soon as possible in all subcomponents (before calling much else) towards getting the fullest timeline of the run. And for this to be "unobtrusive"
fwiw, I think master_task and mype==0 are really the same since
There was a problem hiding this comment.
I guess these lines could be moved up to SetServices from InitializeAdvertise if there is really strong preference
CICE/cicecore/drivers/nuopc/cmeps/ice_comp_nuopc.F90
Lines 346 to 368 in fa682b3
There was a problem hiding this comment.
I think it is not guaranteed on all systems that master_task = 0. Does the call to ufs_trace_init really have to happen at this phase? I realize there are often issues with circular dependencies. I feel like the tracing doesn't need to happen before the scatter and PE decomposition.
There was a problem hiding this comment.
I think it's just easier to trace fully than "be smart" about what needs to be traced
One example where accounting/separating the different phases was found to be very useful was in trying to reduce GFS initialization time (ufs-community/ufs-weather-model#2831). Like removing some advertised fields from FV3 reduced InitializeAdvertise (top to bottom)

| character(*), parameter :: u_FILE_u = & | ||
| __FILE__ | ||
|
|
||
| integer :: mype = -1 |
There was a problem hiding this comment.
There is already my_task available in the code.
I think both CESM and UFS have benefited greatly from sharing common nuopc_caps between MOM6, CICE6 and WW3, coupled to CMEPS (and CDEPS). I think we should strive to maintain that inter-operability. But when there are features or options that are really used by only one system, I think we all benefit from making those use cases as unobtrusive as possible, thus the existence of various wrapper mods in the shared NUOPC caps. That was my thinking in asking Nick to get feedback from the other cap users. The ufs-tracing was something one of our colleagues developed during the recent push to get the GFSv17 ready for operations. The existing EMSF based profiling didn't provide the sort of "time line" feature he wanted, so he developed a very basic tracing code that could sit inside each component. I think it is an exceptionally useful little feature, but it would need a CESM user to enable it across their infrastructure system (driver etc). |
@anton-seaice Not afaik, but it's intended to be a straightforward setup. Each component writes to its own trace file and then combine them (ufs_tracing/combine_traces.sh) to look like: Then, can open with Perfetto UI online tool (https://ui.perfetto.dev/) and navigate wasd |
For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers
PR checklist
Add UFS tracing instrumentation to nuopc/cmeps driver
@DusanJovic-NOAA
No changes in UFS regression tests (Add tracing instrumentation ufs-community/ufs-weather-model#2884)
ufs-community/ufs-weather-model#2884 adds a simple tracing module to UFS and updates some subcomponents' nuopc drivers to produce a trace file which can be used to identify performance issues. The tracing module is not built and used by default. It can be enabled by setting a build option `-DUFS_TRACING=ON'. The generated trace files can be visualized using the chrome-tracing tool or, for example, the Perfetto UI online tool (https://ui.perfetto.dev/) like in ufs-community/ufs-weather-model#2883 . Tracing has been useful in optimizing GFS runtime.