-
Notifications
You must be signed in to change notification settings - Fork 24
Add tracing instrumentation to nuopc driver #151
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
base: emc/develop
Are you sure you want to change the base?
Add tracing instrumentation to nuopc driver #151
Conversation
call ESMF_GridCompGet(gcomp, vm=vm, rc=rc) | ||
if (ChkErr(rc,__LINE__,u_FILE_u)) return | ||
call ESMF_VMGet(vm, localPet=localPet, rc=rc) | ||
if (ChkErr(rc,__LINE__,u_FILE_u)) return | ||
maintask = .false. | ||
if (localPet == 0) maintask=.true. | ||
#ifdef UFS_TRACING | ||
if (maintask) call ufs_trace_init() | ||
if (maintask) call ufs_trace("cmeps", "SetServices", "B") | ||
#endif | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should all of this be in a UFS_TRACING ifdef?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting vm and localPet are not strictly related to tracing. I actually copied that code from InitializeP0 in order to have maintask defined here. Maybe this block can be removed from InitializeP0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have a wrapper mod in CMEPS, but I wonder if the med_utils_mod
could be used for this purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest that you introduce a new file ufs_trace_wrapper.F90 with ifdef blocks:
#ifdef UFS_TRACE
use ufs_trace_mod, only: ufs_trace
#endif
contains
subroutine ufs_trace_wrapper(a, b, c)
character(len=*), intent(in) :: a, b, c
#ifdef UFS_TRACE
call ufs_trace(a, b, c)
#endif
return
end subroutine
end module
Then call ufs_trace_wrapper in the code instead of having ifdef all over the place.
@jedwards4b Please take a look again. I introduced the wrapper module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMEPS standard requires that you use the only clause with all module use statements.
Otherwise this looks good. Thanks.
mediator/med.F90
Outdated
use med_phases_profile_mod , only : med_phases_profile_finalize | ||
use shr_log_mod , only : shr_log_error | ||
|
||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the only clause.
mediator/med_phases_history_mod.F90
Outdated
use perf_mod , only : t_startf, t_stopf | ||
use pio , only : file_desc_t | ||
use shr_log_mod , only : shr_log_error | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add an only clause
mediator/med_phases_ocnalb_mod.F90
Outdated
use shr_orb_mod , only : shr_orb_cosz, shr_orb_decl | ||
use shr_orb_mod , only : shr_orb_params, SHR_ORB_UNDEF_INT, SHR_ORB_UNDEF_REAL | ||
use shr_log_mod , only : shr_log_unit, shr_log_error | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the only clause
mediator/med_phases_post_atm_mod.F90
Outdated
use med_utils_mod , only : chkerr => med_utils_ChkErr | ||
use med_internalstate_mod , only : compocn, compatm, compice, complnd, compwav | ||
use perf_mod , only : t_startf, t_stopf | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the only clause
mediator/med_phases_post_ice_mod.F90
Outdated
use med_phases_history_mod, only : med_phases_history_write_comp | ||
use med_internalstate_mod , only : compice, compocn, compwav | ||
use perf_mod , only : t_startf, t_stopf | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the only clause
mediator/med_phases_prep_atm_mod.F90
Outdated
use perf_mod , only : t_startf, t_stopf | ||
use med_phases_aofluxes_mod, only : med_aofluxes_map_xgrid2agrid_output | ||
use med_phases_aofluxes_mod, only : med_aofluxes_map_ogrid2agrid_output | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add only clause
mediator/med_phases_prep_ice_mod.F90
Outdated
use med_internalstate_mod , only : coupling_mode | ||
use esmFlds , only : med_fldList_GetFldListTo | ||
use perf_mod , only : t_startf, t_stopf | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add only clause
mediator/med_phases_prep_ocn_mod.F90
Outdated
use esmFlds , only : med_fldList_GetfldListTo, med_fldlist_type | ||
use med_internalstate_mod , only : compocn, compatm, compice, coupling_mode | ||
use perf_mod , only : t_startf, t_stopf | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add only clause
mediator/med_phases_prep_wav_mod.F90
Outdated
use esmFlds , only : med_fldList_GetfldListTo | ||
use med_internalstate_mod , only : compatm, compwav | ||
use perf_mod , only : t_startf, t_stopf | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add only clause
mediator/med_phases_restart_mod.F90
Outdated
use shr_is_restart_fh_mod , only : log_restart_fh | ||
#endif | ||
use shr_log_mod , only : shr_log_error | ||
use med_ufs_trace_wrapper_mod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add only clause
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you!
Description of changes
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.
Specific notes
Contributors other than yourself, if any:
CMEPS Issues Fixed (include github issue #):
Are changes expected to change answers? (specify if bfb, different at roundoff, more substantial)
b4b
Any User Interface Changes (namelist or namelist defaults changes)?
No
Testing performed
Please describe the tests along with the target model and machine(s)
If possible, please also added hashes that were used in the testing