Add tracing instrumentation to nuopc driver #103
Conversation
|
@DusanJovic-NOAA CICE has a |
|
|
@DusanJovic-NOAA Same question as for WW3---could you put this in the wrapper mod and avoid the ifdefs in the cap? |
OK, I tried adding the stub version of the 'ufs_trace_mod' in diff --git a/cicecore/drivers/nuopc/cmeps/cice_wrapper_mod.F90 b/cicecore/drivers/nuopc/cmeps/cice_wrapper_mod.F90
index db0140b..7532791 100644
--- a/cicecore/drivers/nuopc/cmeps/cice_wrapper_mod.F90
+++ b/cicecore/drivers/nuopc/cmeps/cice_wrapper_mod.F90
@@ -101,3 +101,28 @@ end subroutine t_barrierf
#endif
end module cice_wrapper_mod
+
+#ifndef UFS_TRACING
+
+module ufs_trace_mod
+
+ implicit none
+ public ufs_trace_init
+ public ufs_trace
+ public ufs_trace_finalize
+
+contains
+
+ subroutine ufs_trace_init()
+ end subroutine ufs_trace_init
+
+ subroutine ufs_trace(component, routine, ph)
+ character(len=*), intent(in) :: component, routine, ph
+ end subroutine ufs_trace
+
+ subroutine ufs_trace_finalize()
+ end subroutine ufs_trace_finalize
+
+end module ufs_trace_mod
+
+#endifAnd removed all Compilation, actually linking, fails with "'multiple definition of `ufs_trace_mod._'" error. As I expected, see my comment in NOAA-EMC/WW3#1494 (comment) |
|
Instead of adding the stub module in the cap, as I tried above, we could add a wrapper module, as we did in CMEPS. The wrapper module will still have ifdefs, while the cap will just call the wrapper routines without ifdefs. |
|
@DusanJovic-NOAA I don't see that you actually converted this to a stub as you did in CMEPS. Did that not work? |
If ifdefs are acceptable in the cap, I prefer to leave it as it is. If not, we can add a wrapper module, but that module will also have ifdefs. Which option is preferred? |
|
The cice_wrapper_mod.F90 has always existed for the purpose of avoiding the ifdefs in the main code. The Consortium itself I believe tries to be agnostic about the individual drivers. @NickSzapiro-NOAA I see you've already approved, so you think the ifdefs (outside of a wrapper) are acceptable to CESM users? |
|
They seem similar to me. Do you want me to start PR at CICE-Consortium (to see)? |
| 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 | ||
|
|
||
| #ifdef UFS_TRACING | ||
| if (mype == 0) call ufs_trace_init() | ||
| if (mype == 0) call ufs_trace("cice", "SetServices", "B") | ||
| #endif |
There was a problem hiding this comment.
A minor suggestion is for all of these lines to be under ifdef UFS_TRACING.
Maybe mype and vm too so not to make unused variable warning for other models
|
There have not been any major objections with this at CICE-Consortium and seems ok |
Add UFS_TRACING ifdef around variables specific to tracing
3 participants
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 tracing instrumentation to nuopc driver
ENTER INFORMATION HERE
ENTER INFORMATION HERE
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.