CMake for reg2grb2 #268
CMake for reg2grb2 #268
Conversation
|
Maybe it's just me, but this seems it could be independent of #264 and doesn't really need to wait for it?In addition, as this does not delete sorc/reg2grb2.fd/Makefile or update the build script sorc/build_reg2grb2.sh and is just adding a file, couldn't this just go in now? |
*Corrected link lib names
|
I did test the file manually putting it into Rahul's CMake work with feature/cmake, and it built the executable. |
|
@BrianCurtis-NOAA Checking in...is this draft PR still needed? Thanks! |
|
Has reg2grb2 been removed or already ported to cmake elsewhere? I believe I was waiting on the CMake build to be ported into coupled-crow. |
|
Yeah missed the cmake port completely. Let me get this added to the sorc CMakeLists.txt. |
|
As long as it builds and runs, it's fine as far as I'm concerned. I don't have any tests to confirm output for reg2grb2. |
|
OK, I'll test on hera and switch this to ready when done. |
|
Moving to ready. |
|
Please hold for now while I check on something. |
|
I left out the find package calls, my reg2grb2 builds OK, but the build on sorc fails with the I'm not sure if you want to get that fixed before merging, as reg2grb2 build succeeds. |
|
Are there no changes needed to build_reg2grb2.sh or reg2grb2.fd/Makefile to use cmake? |
|
@WalterKolczynski-NOAA OK so the build issue above was actually an issue with hpc-stack that I got taken care of (at least on hera). Make sure that the ncio build from hpc-stack has a module_ncio.mod and not module_fv3gfs_ncio.mod. The script and the makefile are no longer needed. The build completes on Hera. |
Now that ncio is available in the hpc-stack, removed the workaround that cloned the ncio repository. Refs: NOAA-EMC#268
Reordered the build workflow utils script so that machine-setup, which determines the target machine, is called before $target is used to determine the appropriate modulefile. Also added error checking so the script exits immediately if the modulefile is missing instead of trying to build. Refs: NOAA-EMC#268
WGRIB2 and landsfcutil are needed to build workflow utils but were not included in the modulefiles. Refs: NOAA-EMC#268
With reg2grb2 added to the workflow utils being built with cmake, the executable needed to be added to the list of executables linked by the linker script. Refs: NOAA-EMC#268
Now that reg2grb2 is part of the workflow_utils cmake build, the old build script is no longer available and a separate entry is no longer needed for build_all. Refs: NOAA-EMC#268
|
@BrianCurtis-NOAA Take a look at the additional changes I made. System now builds using build_all and link_fv3gfs creates the appropriate link to the executable in the exec directory. |
|
Looks good to me! Thanks. |
| @@ -5,35 +5,19 @@ set -eux | |||
| readonly UTILS_DIR=$(cd "$(dirname "$($cmd -f -n "${BASH_SOURCE[0]}" )" )" && pwd -P) | |||
|
|
|||
| # Adapt for global-workflow structure. | |||
| target=${target:-"NULL"} | |||
| source ${UTILS_DIR}/machine-setup.sh > /dev/null 2>&1 | |||
There was a problem hiding this comment.
why is this required?
which mahcine is requiring this to be sourced that supports the global workflow?
There was a problem hiding this comment.
When I tried running build_workflow_utils stand-alone, $target isn't defined until it is set by machine-setup, so it was using NULL, then quietly not loading a modulefile because there wasn't one for NULL, then cmake would fail.
| else | ||
| echo "FATAL: modulefile $modulefile not found!" | ||
| exit 1 |
There was a problem hiding this comment.
This assumes that on every machine there has to be a modulefile.
If the software stack is in path, a modulefile is not required.
Which machine is this failing on?
There was a problem hiding this comment.
The need for this came to me based on the above comment. In the context of global-workflow, I don't know that manually loading a module beforehand is a use case to support, and I'd prefer the error to specify the actual problem rather than cmake just failing.
| landsfcutil::landsfcutil_d | ||
| ip::ip_d | ||
| sp::sp_d | ||
| bacio::bacio_4 | ||
| w3nco::w3nco_4 |
There was a problem hiding this comment.
has anyone verified that all of these are actually used in this program?
There was a problem hiding this comment.
I went through the makefile and compared which ones were being linked. I didn't go through the code line by line.
There was a problem hiding this comment.
That's a question for @BrianCurtis-NOAA, but landsfcutil is a new dependency for this PR, so I assume yes for that at least.
There was a problem hiding this comment.
@aerorahul the old build_reg2grb2.sh has landsfcutil in a LIBS export which was then used in the makefile that was used to build reg2grb2. I don't see any subroutines that seem to be using any landsfcutil subroutines, but if needed I will spend more time on going line by line to see if i missed something. Let me know.
|
@aerorahul Unless you want me to hold this, I'd like to get it merged. |
Please go ahead. |
…itial, ocn -> wat, merra2 threading (NOAA-EMC#279) * changed .gitmodules to point to merra2 ccpp/physics * remove GFDL_atmos_cubed_sphere and ccpp-framework from .git module * remove IPD gfsphysics * Update .gitmodules and submodule pointer for ccpp-physics for code review and testing * Remove interstitial zorl composites * Update .gitmodules and submodule pointer fpor ccpp-physics for code review and testing * Remove or replace references to IPD in comments in atmos_model.F90 * Initialize Sfcprop%zorlx to clear_val instead of huge * Update submodule pointer for ccpp-physics * Rename Fortran variables and CCPP standard names / long names of surface composites from ocean to water * Rename Sfcprop%zorlw to Sfcprop%zorlwav * Rename Sfcprop%zorlo to Sfcprop%zorlw * update submodule pointer for ccpp-physics * Revert change to .gitmodules and update submodule pointer for ccpp-physics Co-authored-by: anning.cheng <anning.cheng@noaa.gov>
Add a CMakeLists.txt to be able to create reg2grb2.x for coupled-crow after the merge of CMake work from #264
Linked issue:
#263
Waiting on:
Pull #264