-
Notifications
You must be signed in to change notification settings - Fork 623
Add tracing instrumentation to nuopc driver #1494
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: dev/ufs-weather-model
Are you sure you want to change the base?
Add tracing instrumentation to nuopc driver #1494
Conversation
@DusanJovic-NOAA Would you be able to make the same sort of changes you made in MOM6, using the wav_wrapper_mod to avoid the ifdefs in the WW3 mesh cap? |
wav_wrapper_mod is a module. Are you suggesting adding a stub version of ufs_trace_mod in the same file? wav_wrapper_mod.F90 is compiled only when UFS_CAP build option is set to "NUOPC_MESH". What if UFS_CAP is not set to "NUOPC_MESH"? |
UWM only uses the mesh cap build option (NUOPC_MESH), so I'm not worried about that at this point. And nobody other than CESM and UFS use the mesh cap. There are other users of the mesh cap (Italians, Australians, Norwegians) but they all build the mesh cap under CESM. For MOM6, you added a small module file w/ a subroutine call to ufs_tracing. I'm not clear on why such a SR call in the wav_wrapper wouldn't be the same type of change? |
I thought that MOM6 will be the only component that does not allow ifdefs in the source code, that's why I added the ufs_stub_mod. Now I see suggestions to use stub module in almost every component, this one, as well as in CICE and CMEPS. We can not have stub 'ufs_trace_mod' module defined in all of them, linker will fail. There must be only one. So the question is which one, MOM6, WW3, CICE or CMEPS. Who will provide the stub module? |
WW3 allows ifdefs in the source code. We have a lot. I know there's been efforts to reduce them, but they can stay if they need to. |
@DusanJovic-NOAA Nobody prefers ifdefs---the mesh cap on the whole does not rely on ifdefs (with the exception of the defunct attempt encapsulated in MOM6 doesn't have a "rule" about ifdefs, since modifications are in the cap code, which is essentially in the hands of those who jointly develop and maintain the caps (drivers). You added it because NCAR asked you to avoid ifdefs if possible. The preference should be for modern development practice (which I trust you on) which minimizes to the extent possible any ifdef use. You are telling me that in this case, that is not possible---to limit in all caps the use of ifdefs and still have the feature useful. |
See my comment in NOAA-EMC/CICE#103 (comment). I know people don't like ifdefs, I don't like them also, nobody likes them, but in this case they are the simplest solution to what we need. We basically need to call a subroutine from each and every component and somehow make sure that the subroutine is available to all components and that it's defined only once, without putting it in some kind of external library and make each and every component depend on it. I thought that by using ifdefs we can do that without disrupting how the components are used outside the ufs-weather-model. Unless UFS_TRACING is defined, nothing is changed, as if nobody touched any source code. I could try to define the stub version in the ufs-weather-model, but then all build systems in all components will need to be updated to conditionally build their own stub version unless they are being compiled as a component of ufs. Which will be an even bigger mess, if possible at all, because what if two of the components are used in some other host model other than UFS, CESM for example. CESM does not have ufs_trace_mod, which means one of the component must provide it. Now, the more I think about this issue, the more I think that getting rid of ifdefs in MOM6 was a mistake. Since the stub version is defined in MOM6 no other component can define it, as long as MOM6 is being linked in the final executable. |
Pull Request Summary
Add tracing instrumentation to nuopc driver
Description
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.
Issue(s) addressed
Commit Message
Check list
Testing