Updates for ccpp-capgen v1

[dustinswales]

Description

This PR makes the necessary changes to the CCPP metadata and Fortran files to support the transition to the new code generator ccpp-capgen v1. This includes:

• Updated CCPP interfaces within the CCPP driver
• Changes to the CMake build system (no more ccpp_prebuild_conf file).
• Metadata file changes and updated suite definition file format

Issue(s) addressed

N/A

Testing

All UFS regression tests pass on Ursa.

Dependencies

• Replace ccpp-prebuild and ccpp-capgen v0 with ccpp-capgen v1 NCAR/ccpp-framework#762
• Updates for ccpp-capgen v1 ufs-community/ccpp-physics#378
• Updates for ccpp capgen v1 NCAR/TEMPO#39
• Metadata bug fixes ufs-community/c3#3
• Updates for ccpp-capgen v1 NOAA-GFDL/GFDL_atmos_cubed_sphere#419
• Updates for ccpp-capgen v1 CMEPS#173
• Updates for ccpp-capgen v1 ufs-community/ufs-weather-model#3288

[dustinswales]

@AlexanderRichert-NOAA Thank you for the suggestion. Testing this now.

[dustinswales]

@gspetro-NOAA @AlexanderRichert-NOAA The GitHub servers are not responding and the CI tests aren't being run.
Should I revert b55730a and we can fix this test later?

[gspetro-NOAA]

@dustinswales No, GitHub Actions was in a degraded state earlier: https://www.githubstatus.com/
That said, since the tests trigger on push or the opening or a pull request, maybe you need to push an empty commit? Or close and reopen? It says the tests run on workflow dispatch, but I only see the option to dispatch it on a ufs-community WM branch, not a fork. I could be missing something tho.

[BrianCurtis-NOAA]

Reading XML file /home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/suites/suite_FV3_GFS_v15_thompson_mynn_lam3km.xml
Suite XML schema version: 1.0
validate_xml_file: xmllint not found, could not validate file /home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/suites/suite_FV3_GFS_v15_thompson_mynn_lam3km.xml

-- Running ccpp_datafile from /home/runner/work/ufsatm/ufsatm/ufsatm/ccpp
Traceback (most recent call last):
'/home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/framework/capgen/ccpp_datafile.py' '--dependencies' '/home/runner/work/ufsatm/ufsatm/build/ccpp/ccpp/datatable.xml'
  File "/home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/framework/capgen/ccpp_datafile.py", line 930, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/framework/capgen/ccpp_datafile.py", line 923, in main
    report = datatable_report(pargs.datatable, action,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/framework/capgen/ccpp_datafile.py", line 737, in datatable_report
    table = _read_datatable(datatable)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/framework/capgen/ccpp_datafile.py", line 233, in _read_datatable
    tree = ET.parse(datatable)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/xml/etree/ElementTree.py", line 1204, in parse
    tree.parse(source, parser)
  File "/usr/lib/python3.12/xml/etree/ElementTree.py", line 558, in parse
    source = open(source, "rb")
             ^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/home/runner/work/ufsatm/ufsatm/build/ccpp/ccpp/datatable.xml'
CMake Error at cmake/ccpp_capgen.cmake:312 (message):
  CCPP file retrieval FAILED: result = 1
Call Stack (most recent call first):
  ccpp/CMakeLists.txt:293 (ccpp_datafile)

[BrianCurtis-NOAA]

Summary of failure

The build fails because ccpp_datafile.py raised FileNotFoundError trying to open: /home/runner/work/ufsatm/ufsatm/build/ccpp/ccpp/datatable.xml
The traceback shows the CMake wrapper ccpp_datafile was invoked and the python script attempted to parse that datatable file but it did not exist.
Likely root causes:
capgen did not produce datatable.xml before ccpp_datafile was run, or
OUTPUT_ROOT / DATATABLE paths passed to ccpp_capgen / ccpp_datafile are incorrect (note the duplicated "ccpp/ccpp" in the path), or
output directory for capgen did not exist so capgen could not write the datatable.
Recommended fixes (apply and test in this order)

Ensure capgen creates its output directory
In cmake/ccpp_capgen.cmake the file(MAKE_DIRECTORY "${arg_OUTPUT_ROOT}") is currently commented out. Create the OUTPUT_ROOT before running capgen so files can be written.
Patch: Replace (around line ~218) #file(MAKE_DIRECTORY "${arg_OUTPUT_ROOT}") with file(MAKE_DIRECTORY "${arg_OUTPUT_ROOT}")
Make sure the OUTPUT_ROOT and DATATABLE paths used by the CMake calls are correct (remove the double "ccpp")
Locate where ccpp_capgen(...) is invoked (ccpp/CMakeLists.txt [line referenced in logs]). Confirm OUTPUT_ROOT is set to the build-side location you expect, e.g. OUTPUT_ROOT "${CMAKE_BINARY_DIR}/ccpp" and that the datatable argument passed to ccpp_datafile points to the same tree, e.g. DATATABLE "${CMAKE_BINARY_DIR}/ccpp/datatable.xml"
If the current invocation uses something like "${CMAKE_BINARY_DIR}/ccpp" plus another "ccpp" or uses ${PARENT_DIR} incorrectly, correct it so there is no duplicated segment. Example change in CMakeLists where these functions are called:
Before (hypothetical/current): ccpp_capgen(... OUTPUT_ROOT "${CMAKE_BINARY_DIR}/ccpp") ccpp_datafile(DATATABLE "${CMAKE_BINARY_DIR}/ccpp/ccpp/datatable.xml" ...)
After (consistent): ccpp_capgen(... OUTPUT_ROOT "${CMAKE_BINARY_DIR}/ccpp") ccpp_datafile(DATATABLE "${CMAKE_BINARY_DIR}/ccpp/datatable.xml" ...)
Add runtime diagnostics and fail-fast checks
Add a check after running capgen to error early if datatable.xml was not produced, with a clear message indicating which step failed. Example (insert after the execute_process calling capgen or in the caller CMakeLists): if(NOT EXISTS "${arg_OUTPUT_ROOT}/datatable.xml") message(FATAL_ERROR "ccpp_capgen did not produce datatable.xml at ${arg_OUTPUT_ROOT}/datatable.xml. Check capgen output and OUTPUT_ROOT argument.") endif()
Improve error capture in ccpp_datafile wrapper so you see python stderr
Modify cmake/ccpp_capgen.cmake ccpp_datafile() execute_process call to capture stderr and include it in the fatal message. Example patch for the function (replace the execute_process block and the error branch):

Replace: execute_process(COMMAND ${CCPP_DATAFILE_CMD} WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" OUTPUT_VARIABLE CCPP_FILES RESULT_VARIABLE RES OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_STRIP_TRAILING_WHITESPACE COMMAND_ECHO STDOUT) With: execute_process(COMMAND ${CCPP_DATAFILE_CMD} WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" OUTPUT_VARIABLE CCPP_FILES ERROR_VARIABLE CCPP_FILES_ERR RESULT_VARIABLE RES OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_STRIP_TRAILING_WHITESPACE COMMAND_ECHO STDOUT)

And replace the error branch: if(RES EQUAL 0) message(STATUS "CCPP files retrieved") else() message(FATAL_ERROR "CCPP file retrieval FAILED: result = ${RES}") endif() With: if(RES EQUAL 0) message(STATUS "CCPP files retrieved") else() message(FATAL_ERROR "CCPP file retrieval FAILED: result = ${RES}\n" "stdout:\n${CCPP_FILES}\n" "stderr:\n${CCPP_FILES_ERR}") endif()

This will make future failures show the python traceback/error output in the CMake log.

Add a temporary status print to confirm the actual paths at configure time
In ccpp_capgen() add: message(STATUS "ccpp_capgen: OUTPUT_ROOT = ${arg_OUTPUT_ROOT}") and in the caller CMakeLists print the datatable path before calling ccpp_datafile: message(STATUS "Calling ccpp_datafile with DATATABLE = ${DATATABLE_PATH}")
Why these changes fix the problem

Creating the output directory avoids capgen failing silently to write files.
Ensuring consistency between OUTPUT_ROOT and the DATATABLE path prevents the double "ccpp/ccpp" mistake that led to the missing file path.
Better diagnostics make the next failure straightforward to diagnose (you will see if capgen failed or if the python script had a different error).
Minimal patch examples

Un-comment directory creation in cmake/ccpp_capgen.cmake:
Change: #file(MAKE_DIRECTORY "${arg_OUTPUT_ROOT}") To: file(MAKE_DIRECTORY "${arg_OUTPUT_ROOT}")
Capture stderr in ccpp_datafile() (snippet):
Change execute_process call to include ERROR_VARIABLE: execute_process(COMMAND ${CCPP_DATAFILE_CMD} WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" OUTPUT_VARIABLE CCPP_FILES ERROR_VARIABLE CCPP_FILES_ERR RESULT_VARIABLE RES OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_STRIP_TRAILING_WHITESPACE COMMAND_ECHO STDOUT)

Improve fatal message on failure: message(FATAL_ERROR "CCPP file retrieval FAILED: result = ${RES}\n" "stdout:\n${CCPP_FILES}\n" "stderr:\n${CCPP_FILES_ERR}")

Immediate testing checklist

Apply the above CMake changes locally or in a feature branch.
Run a clean configure/build:
cmake -S . -B build
cmake --build build -jN
Observe configure logs for the STATUS messages showing OUTPUT_ROOT and DATATABLE path.
If the job still fails, copy the captured stdout/stderr from the ccpp_datafile fatal message and re-run debugging. The improved error message will show whether capgen failed to run or whether the python script has other issues.

[dustinswales]

@BrianCurtis-NOAA @gspetro-NOAA
I really don't feel comfortable debugging this CI test in the middle of the merge process. I would hate to fix the CI and break the UWM. I would rather have a follow up PR that fixes the CI, rather than a follow up to fix the UWM.
Can we merge and revisit?

[climbfuji]

@BrianCurtis-NOAA @gspetro-NOAA I really don't feel comfortable debugging this CI test in the middle of the merge process. I would hate to fix the CI and break the UWM. I would rather have a follow up PR that fixes the CI, rather than a follow up to fix the UWM. Can we merge and revisit?

Here's the smoking gun:

Suite XML schema version: 1.0
validate_xml_file: xmllint not found, could not validate file /home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/suites/suite_FV3_GFS_v15_thompson_mynn_lam3km.xml

If xmllint isn't available, capgen won't run to completion. It seems to fail silently in your setup, because the subsequent ccpp_validator shouldn't get called.

I hate this PR blocking the UFS commit queue and other models (SCM, NEPTUNE) are waiting for the UFS merge to complete, too. I support merging this and I am happy to help fix the CI tests immediately afterwards in a follow-up PR.

[gspetro-NOAA]

@BrianCurtis-NOAA @gspetro-NOAA I really don't feel comfortable debugging this CI test in the middle of the merge process. I would hate to fix the CI and break the UWM. I would rather have a follow up PR that fixes the CI, rather than a follow up to fix the UWM. Can we merge and revisit?

That's @BrianCurtis-NOAA 's call, since he's managing UFSATM merges while Dusan's on annual leave. I'm fine with either, as long as the fix comes in quickly, but I'd suggest reverting b55730a if we aren't going to debug it now.

Also, just a note that there still seems to be an extra ufsatm (3 instead of 2) in the path with the most recent CI update, so we're down from 4, but that may still be a source of problems:

validate_xml_file: xmllint not found, could not validate file /home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/suites/suite_FV3_GFS_v15_thompson_mynn_lam3km.xml

[climbfuji]

@BrianCurtis-NOAA @gspetro-NOAA I really don't feel comfortable debugging this CI test in the middle of the merge process. I would hate to fix the CI and break the UWM. I would rather have a follow up PR that fixes the CI, rather than a follow up to fix the UWM. Can we merge and revisit?

That's @BrianCurtis-NOAA 's call, since he's managing UFSATM merges while Dusan's on annual leave. I'm fine with either, as long as the fix comes in quickly, but I'd suggest reverting b55730a if we aren't going to debug it now.

Also, just a note that there still seems to be an extra ufsatm (3 instead of 2) in the path with the most recent CI update, so we're down from 4, but that may still be a source of problems:

validate_xml_file: xmllint not found, could not validate file /home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/suites/suite_FV3_GFS_v15_thompson_mynn_lam3km.xml

A note on reverting b55730a. That's a nightmare, because you also have to revert every single, recursive submodule update, or otherwise you end up being blocked by stacking new changes on top of code that doesn't compile or run. We are talking about the FV3 dycore, ccpp-physics, ccpp-physics -> tempo, ccpp-physics -> c3, at the minimum.

[dustinswales]

@BrianCurtis-NOAA @gspetro-NOAA I really don't feel comfortable debugging this CI test in the middle of the merge process. I would hate to fix the CI and break the UWM. I would rather have a follow up PR that fixes the CI, rather than a follow up to fix the UWM. Can we merge and revisit?

That's @BrianCurtis-NOAA 's call, since he's managing UFSATM merges while Dusan's on annual leave. I'm fine with either, as long as the fix comes in quickly, but I'd suggest reverting b55730a if we aren't going to debug it now.
Also, just a note that there still seems to be an extra ufsatm (3 instead of 2) in the path with the most recent CI update, so we're down from 4, but that may still be a source of problems:

validate_xml_file: xmllint not found, could not validate file /home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/suites/suite_FV3_GFS_v15_thompson_mynn_lam3km.xml

A note on reverting b55730a. That's a nightmare, because you also have to revert every single, recursive submodule update, or otherwise you end up being blocked by stacking new changes on top of code that doesn't compile or run. We are talking about the FV3 dycore, ccpp-physics, ccpp-physics -> tempo, ccpp-physics -> c3, at the minimum.

@climbfuji b55730a was added after the submodules were merged, so we could revert it w/o issue. No?

[DeniseWorthen]

Does anyone have a sense of how long it might take to fix the issue (the missing xmllint, if understand correctly).

[climbfuji]

@BrianCurtis-NOAA @gspetro-NOAA I really don't feel comfortable debugging this CI test in the middle of the merge process. I would hate to fix the CI and break the UWM. I would rather have a follow up PR that fixes the CI, rather than a follow up to fix the UWM. Can we merge and revisit?

That's @BrianCurtis-NOAA 's call, since he's managing UFSATM merges while Dusan's on annual leave. I'm fine with either, as long as the fix comes in quickly, but I'd suggest reverting b55730a if we aren't going to debug it now.
Also, just a note that there still seems to be an extra ufsatm (3 instead of 2) in the path with the most recent CI update, so we're down from 4, but that may still be a source of problems:

validate_xml_file: xmllint not found, could not validate file /home/runner/work/ufsatm/ufsatm/ufsatm//ccpp/suites/suite_FV3_GFS_v15_thompson_mynn_lam3km.xml

A note on reverting b55730a. That's a nightmare, because you also have to revert every single, recursive submodule update, or otherwise you end up being blocked by stacking new changes on top of code that doesn't compile or run. We are talking about the FV3 dycore, ccpp-physics, ccpp-physics -> tempo, ccpp-physics -> c3, at the minimum.

@climbfuji b55730a was added after the submodules were merged, so we could revert it w/o issue. No?

Correct, but you can't work on/merge any other PRs that require updates to any of the submodules that were already modified for this PR.

[climbfuji]

Does anyone have a sense of how long it might take to fix the issue (the missing xmllint, if understand correctly).

Probably less than this entire conversation.

[climbfuji]

Does anyone have a sense of how long it might take to fix the issue (the missing xmllint, if understand correctly).

Probably less than this entire conversation.

Sigh, I'll try.

[BrianCurtis-NOAA]

Does anyone have a sense of how long it might take to fix the issue (the missing xmllint, if understand correctly).

Probably less than this entire conversation.

Sigh, I'll try.

I'm trying too, but i haven't built the stack in my own fork, so i have to wait on stack to build and the CI to fail before testing fixes.

[climbfuji]

See #1130, looking promising so far

[climbfuji]

@dustinswales My attempt seems to be working (#1130). Please merge dustinswales#10 into your branch to update this PR, and we should be good to go.

[dustinswales]

FYI @gspetro-NOAA @BrianCurtis-NOAA @climbfuji I will be out of the office this afternoon.
I'm leaving the office now, but if this get merged soon, I can update when I get home. Unless someone else can do this?

[BrianCurtis-NOAA]

Thanks @climbfuji @dustinswales for working with us on resolving this.

We're going to run compiles only on the UFSWM rt.sh on only one machine to make sure we didn't break anything with that PARENT_DIR change.

[gspetro-NOAA]

A minor CMEPS change was needed at the WM level (thanks to @NickSzapiro-NOAA), but now all RTs are passing on Ursa (new log uploaded), and the UFSATM CI is passing. 🎉

[climbfuji]

A minor CMEPS change was needed at the WM level (thanks to @NickSzapiro-NOAA), but now all RTs are passing on Ursa (new log uploaded), and the UFSATM CI is passing. 🎉

Thanks for your efforts on this over the weekend @gspetro-NOAA. Looks like we can finally merge this?

[dustinswales]

@gspetro-NOAA Thank you!

[BrianCurtis-NOAA]

Thanks everyone for taking the time to ensure this is done the right way. I know we want to push the ideal that CI should pass before we start testing, and the process to fix CI ended up delaying things. I know I will continue to adjust to ensure the process is smoother. Dusan does some great work in UFSATM and I definitely wanted to make sure he comes back to his repo without issue.