Skip to content

Conversation

DusanJovic-NOAA
Copy link
Contributor

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

  • How were these changes tested?
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)?
  • Please indicate the expected changes in the regression test output, (Note the list of known non-identical tests.)
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):

@DeniseWorthen
Copy link
Contributor

@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?

@DusanJovic-NOAA
Copy link
Contributor Author

@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"?

@DeniseWorthen
Copy link
Contributor

DeniseWorthen commented Sep 22, 2025

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?

@DusanJovic-NOAA
Copy link
Contributor Author

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?

@JessicaMeixner-NOAA
Copy link
Collaborator

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.

@DeniseWorthen
Copy link
Contributor

DeniseWorthen commented Sep 22, 2025

@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 wav_shr_flags). Those that are there are there primarily because the wave code itself requires them.

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.

@DusanJovic-NOAA
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants