Update cmake documentation, remove old build scripts #45
Conversation
… right gfortran flags for everything to compile correctly
mkavulich
requested review from
gsketefian,
JeffBeck-NOAA and
JulieSchramm
as code owners
There was a problem hiding this comment.
@mkavulich Questions from Julie's email:
Will you be removing the "git clone" and "manage_externals/checkout_externals" commands from the README files for the build? We discussed it at the documentation meeting, and thought it would be easier to document and do training if users could just use all of the commands in the file. Would it be possible to make it an executable, so users could just run the script to set up the environment and run the cmake and make commands?
|
@jwolff-ncar those commands are removed from the README files and consolidated in the single INSTALL file. The READMEs now only contain environment variables Regarding making the READMEs executable, in their current form they can be sourced or run as scripts, but I would rather not advertise that capability since that will require testing and maintaining each individual file on a regular basis. Additionally, I'd rather users not get into the idea that we have magic scripts that will set up their environment for them: with that black box behavior we might as well just maintain a monolithic build script, which is a philosophy we are trying to get away from. All that said, I'm willing to hear counter-arguments. |
|
@mkavulich, Does the cmake build also handle copying all executables to the /bin directory? I see that the install script was removed, so I was curious to know how that's handled now. |
I have to say I still don't understand why creating all this extra work for the user (and us) is a better way to go than having a build script that works for most users. You still have to maintain these READMEs, so why not make them executable? We have to maintain a lot of machine-dependent code in the regional_workflow scripts anyway. Is the idea to teach the user how it all works? If there was a list of clear arguments for this approach, maybe it will remind me of an advantage I'm not currently seeing. Also, not having these be executable forces us to maintain run-time module files in regional_workflow (in modulefiles/tasks/hera, for example). If these were executable, we could source them and avoid such hard-coding. |
|
@mkavulich How does this new build system know which suites to build FV3 for? Is it getting the list from the list of valid values in regional_workflow/ush/valid_param_vals.sh? |
My initial proposal was going to be to do exactly that (a top-level script that sources the READMEs), and I even developed a version to show off, but there is a lot of resistance to the idea of supporting an "official" build script. I agree in some aspects: I do not want to support one to the community that will almost definitely not work out of the box for most machines. But I understand what you (and Jeff) have been saying about making more work for ourselves: for us testers (who will eventually be in the vast minority of all users) it is tremendously annoying to have to do several manual steps every time you build the model. Heck, it would be easier for me as well. A build script would also be desirable for automated testing. So here is my final proposal: I will open a separate PR that includes a build script--the one that I developed earlier but never shared the details of. It will be quite simple and simply take a platform and (optional) compiler argument, and source the README files which will remain in valid bash format as they are now. In this way, we will not need to maintain two different systems, but merely a script on top of the official, documented build system. However, we will not include this build script in any public documentation (I'd prefer to not have it in the released code at all, but I don't think that's easy to do if we're releasing through github). So it will only be for use in testing and internal development. I'll tag the build team members that were especially vocal to make sure that we don't blindside anyone who might still have arguments against this idea. Does that sound like a reasonable path forward? @JeffBeck-NOAA @gsketefian @jwolff-ncar @JulieSchramm @llpcarson @whoeverelsemightbeinterestedinthis |
@JeffBeck-NOAA the cmake flag "-DCMAKE_INSTALL_PREFIX=" sets the install location of the bin/ directory. We instruct users to set "-DCMAKE_INSTALL_PREFIX=.." so this directory ends up one up from where the code is built, which puts bin/ at the top level of the app.
@gsketefian I just opened a separate PR (#47) that sets the two released suites as default, and allows the user to specify a different set at build time if they wish. We should apply a similar change to the develop branch, but maybe building all the suites by default? |
|
Thanks for trying to please everyone, @mkavulich. An impossible task. We just need to close this out for this release and then take up the arguments again at a later time if necessary. |
|
@mkavulich, thanks for coming up with a compromise for the build script. I'm hoping this will satisfy those who were against a fully-automated build. Question to @mkavulich and @gsketefian, can't we still source the README files at run-time, even if they're not executable? |
@JeffBeck-NOAA That is correct, they can be sourced, but only for bash-like shells since they use |
|
@mkavulich Your proposal to have a build script that we won't advertise is ok with me! Also important will be the capability to source the READMEs during runtime. Otherwise, we'll have to maintain separate module files for run-time. It's ok if they only work in bash because the J-job and ex-scripts are all bash. Thanks. @JeffBeck-NOAA |
|
@mkavulich As for which suites to build for, it's fine if the user specifies them on the command line, although if they don't specify, then (1) how do you get the list of all suites (is that somewhere in the FV3 code?), and (2) building for all suites might take a very long time (I've never tried though). Also, the workflow needs to know what suites the build used so that it can immediately flag if the user specified a suite that wasn't included in the build. That's what the parameter valid_vals_CCPP_PHYS_SUITE in valid_param_vals.sh is for. My approach thus far has been to have valid_vals_CCPP_PHYS_SUITE specify the set of suites to build for. I guess we can take this parameter to define the "all" case. But if the user specifies a custom set of suites to build for, how do we get that information to the workflow? |
|
@gsketefian For the release branch (this PR) I think it is simple enough to hard-code valid_vals_CCPP_PHYS_SUITE to only accept the two supported release suites. If a user is trying to use an unsupported suite, they should be working on the I agree we need a more robust solution than hard-coding when this is applied to |
|
@mkavulich Sure, no problem. Good luck with the training. |
Update cmake documentation, remove old build scripts (ufs-community#45) The new cmake build requires the user to set up the environment specific to their platform. These requirements are now documented in a series of README_* files that the user can follow along with for setting their environment. In addition, this removes the old build system scripts to avoid confusion.
* Update compile envirnment for DA on Jet. * Update compile envirnment for DA on Hera. * RRFS_dev2: update compile on WCOSS and Orion (ufs-community#45) * Update compile envirnment for DA on Jet, Hera, WCOSS (Dell), and Orion. * Use hash for GSI and rrfs_util instead of branch. * Update UFS_UTIL hash from 41a2ac8 to fae3ae8
…/framework submodule pointer update for ufs-community#462 (#1654) * update FV3 submodule and .gitmodules for testing of 20230313_combo * turn off cpld_control_p8_faster cheyenne
DESCRIPTION OF CHANGES:
The new cmake build requires the user to set up the environment specific to their platform. These requirements are now documented in a series of README_* files that the user can follow along with for setting their environment.
In addition, this PR would remove the old build system scripts to avoid confusion.
TESTS CONDUCTED:
Built successfully on all of the platforms with associated README files, save Cheyenne (gnu) and MacOSX. I will iterate on these instructions until I can get a successful build.
Out of an abundance of caution, also ran a series of end-to-end run tests on Hera; all of which ran successfully:
ISSUE (optional):
Resolves #40, will hopefully resolve #37 once I figure out the correct flags/settings