-
Notifications
You must be signed in to change notification settings - Fork 180
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
Update feature/coupled-crow to run the latest ufs-weather-model with prognostic aerosols #338
Update feature/coupled-crow to run the latest ufs-weather-model with prognostic aerosols #338
Conversation
prognostic aerosols into the UFS, replacing original FV3-GOCART system. A forked repository is temporarily used to download the updated ufs-weather-model app.
app with prognostic aerosols.
with latest FV3 revision and include settings for FV3_GFS_v16 and aerosol scavenging factors.
This item is implemented as a YAML list of strings, which allows to set scavenging factors for each tracer.
removing all references to `cplgocart`.
- Copy NUOPC field dictionary to run directory by default.
While I'm not necessarily against the changes, there are important changes here (like changing the modulefile used by global-workflow at runtime) that really should've been discussed with the global-workflow team before being implemented. Additionally, the module name change impacts the non-aerosol version, which is still using the old UFS naming convention. Finally, with regard to the modulefile, I don't know that UFS is going to want to keep a separate modulefile for aerosols, given that no other component has its own modulefile in the root modulefile directory. The change from cplgocart to cplchem in CROW is also going to conflict with PR #334, which makes changes keyed off of cplgocart that will need to be updated. On the whole, I'd prefer to sit on this one if possible at least until the model changes are merged into the authoritative ufs-weather-model repo and we can coordinate with @JessicaMeixner-NOAA to update the entire coupled workflow to the latest UFS commit at the same time. Ideally merging to a single build command with all the components compiled as well, instead of different build settings. |
Since this only partially addresses #324 I'm afraid if we merge this by itself we will break the ATM-OCN-ICE-WAV model workflow. However, if we only address #324 without this, we'd likely break the aerosols app as well. So perhaps these will have to be combined into an effort. I'm still not sure who is going to address #324 in full but hopefully we can discuss that at the tag-up here shortly. @WalterKolczynski-NOAA Depending on how long it will take to fully reintegrate GOCART into ufs-weather-model, I don't think waiting until it's one build system should stop us. However, if we're changing modules for the ATM-OCN-ICE-WAV as well, we really need to consider how that impacts. @rmontuoro did you run the coupled_free_forecast_wave.yaml case? Perhaps I'm thinking it conflicts with #324 when it actually doesn't? |
sorc/build_ufs_coupled.sh
Outdated
|
||
module purge | ||
module use $MOD_PATH | ||
module load fv3 | ||
module load ufs_${target} | ||
cd ufs_coupled.fd/ | ||
if [[ -d build ]]; then rm -Rf build; fi | ||
if [[ -d GOCART ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you have switched to using your fork, but as soon as this PR goes into ufs-weather-model this build check will not work anymore. Do we address that now or later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the most recent versions of ufs-weather-model
all modulefiles have been renamed as: ufs_<machine>.<compiler_settings>
. This change introduces the update and should work from now on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you didn't update the hash of what is checked out for the ufs-weather-model for the coupled case. Therefore this will break the coupled case as is, unless I missed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was my issue as well. The currently non-aerosol coupled-crow ufs model version is still using the old naming convention. So this PR would break that until #324 is also merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(And vice versa)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. My assumption was that we were going to update to the latest ufs-weather-model for the coupled case, too, before or at the same time as this PR. I will address this issue by adding a conditional block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope we will update ufs-weather-model for the other case very soon, but as of now #324 is remains an unassigned task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit f81741b addresses this issue. Updated modulefiles are only loaded to build the ufs-weather-model with prognostic aerosols. This model is identified though a hidden file, created when using option -a
during checkout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I'm too late and the complications have already been added, but I'm going to try and tackle #324 after we clear a couple of the other PRs out. So let's cool it with adding more if-block exceptions for aerosols when we want to be removing them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least until we can discuss it in the afternoon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a few comments. Before this is merged, we need to confirm that the atm-ocn-ice-wav case still runs as is.
ush/load_fv3gfs_modules.sh
Outdated
@@ -24,10 +24,10 @@ elif [[ -d /lfs3 ]] ; then | |||
module load module_base.jet | |||
elif [[ -d /scratch1 ]] ; then | |||
# We are on NOAA Hera | |||
module load module_base.hera | |||
module use "$HOMEgfs/sorc/ufs_coupled.fd/modulefiles" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't necessarily disagree with this change, but @WalterKolczynski-NOAA needs to be okay with it. I don't think there are other modules that are needed when running the forecast but I could be wrong. This is just for the forecast job right? If it's more than just the forecast I don't think this will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fully defer to your choice here. My assumption was that platform-specific modulefiles provided with the source code would include all necessary modules to run different coupled configuration but this may not be the case.
While this is mostly a convenience-driven change, I'm concerned about duplicating model settings in the workflow as it will make it harder to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global-workflow team has already discussed moving to this paradigm (using the same script for build and runtime), but haven't gotten to it yet. Here I want to be sure that replacing base with the UFS module here is only impacting the forecast job and not any other jobs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit 0307896 refactors this script to:
- use a new shell function (
get_platform
) introduced to return the string identifier for the auto-detected local platform to improve modularity - load updated modulefiles only for coupled aerosols on Hera/Intel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit a821a1a fixes additional issues with the script.
ush/load_fv3gfs_modules.sh
Outdated
if [[ -d $HOMEgfs/sorc/ufs_coupled.fd/GOCART ]] ; then | ||
module use "$HOMEgfs/sorc/ufs_coupled.fd/modulefiles/hera.intel" | ||
module load fv3 | ||
module load ufs_aerosols_hera.intel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like above, this works for now, but this will not be sufficient after the PR goes in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This modulefile should go away soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is the if the GOCART directory exists is not going to work once the ufs-weather-model PR is committed and all scenarios have the folder GOCART. So this will have to not adversely affect the other scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. We should limit use of the additional aerosol modulefile only to coupled configurations building GOCART.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Work is ongoing to include all GOCART dependencies into hpc-stack.
@*rmontuoro *All the UFS applications require this yaml file including the
standalone FV3 since May 21 So it's good to keep the yaml file without any
constraints.
…On Wed, Jun 9, 2021 at 4:02 PM Raffaele Montuoro ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In ush/nems_configure.sh
<#338 (comment)>
:
> fi
+cp $HOMEgfs/sorc/ufs_coupled.fd/tests/parm/fd_nems.yaml fd_nems.yaml
This is an interesting question.
Yes, the NUOPC field dictionary is not required by standalone atm.
Furthermore, recent FV3 versions no longer rely on the cpl flag, formerly
in the model_configure file, to advertise/realize coupling fields in the
NUOPC cap. We could wrap this file copy in a conditional block to check
whether any of the physics coupling flags (cplflx, cplwav, etc.) are
true, similarly to what implemented in cplvalidate.sh:
https://github.com/NOAA-EMC/global-workflow/blob/3428097dd39934ea648d86e5cdee8d8c2bdb3b14/ush/cplvalidate.sh#L32
However, there is an ongoing effort to refactor the FV3 cap, with the
possible (but remote) outcome that other coupling flags may no longer be
used, too.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#338 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI7D6TNJT6PCYBGW7UTT46DTR7CERANCNFSM46LHFZVQ>
.
|
ufs-weather-model with prognostic aerosols. This option is recognized by means of a hidden file created in the model's repository when running checkout.sh with option -a.
A new shell function (get_platform) is introduced to increase modularity. This fuction auto-detects the local platform and returns a string identifier. As a result, load_fv3gfs_modules.sh is refactored.
generate the FV3 namelist file for FV3-GOCART.
@JessicaMeixner-NOAA - the |
…ture/update-aerosols
…ture/update-aerosols
Adds the ALLOW_LANDMASK_CHANGES option to the MOM6 input templates and allows the setting via MOM6_ALLOW_LANDMASK_CHANGES (default False). Refs: NOAA-EMC#324
The newest version of CICE replaces istep0 with the normal month and day, so the namelist is updated to reflect that. ktherm and tfrz_option are updated to allow alternate settings, following similar changes to the UFS test harness. The default values are the previous settings. Settings for tr_FY, sw_redist, f_mlt_onset, and f_frx_onset are updated to new values in the UFS test harness. Additionally, many namelist options have been removed following the same deletions in the UFS test harness. Refs: NOAA-EMC#324
Some changes to the UFS require updates to the build script. The modulefile organization has been updated to be modulefiles/ufs_${target} rather than modulefiles/${target}/fv3, so the module paths in the build script are updated correspondingly. A cmake switch DAPP is now required in order to build UFS. Non- aerosol coupled is using S2SW for the time being. This replaces setting DS2S and DWW3 to YES. The old v15 suite is removed from the CCPP suite list. Refs: NOAA-EMC#324
The location of the fd_nems.yaml file changed in the UFS repository, so updated nems_configure to copy from the new location. Refs: NOAA-EMC#324
The latest version of UFS adds a setting to define the ocean mesh file, so the nems.configure templates are updated to define them. They are identical to the ice mesh file, so the MESHICE variable is renamed to MESH_OCN_ICE and used for both. Additionally, the use_mommesh variable is added and defaults to true. Refs: NOAA-EMC#324
The WW3 pre and post components now need NetCDF to build, so NetCDF is added back to the modulelist, and previously removed code in the build script is restored. Refs: NOAA-EMC#324
In the latest UFS, the mediator name was changed from nems to cmeps, so the nems.configure is updated to match. Refs: NOAA-EMC#324
Update wave IC to new set compatible with current UFS version. Refs: NOAA-EMC#324
Update module versions to match those in the UFS common file. Refs: NOAA-EMC#324
There were still a number of settings in the ice input file that did not match the setting in UFS (and one setting that was misplaced). Refs: NOAA-EMC#324
GitHub repository for the UFS weather model only. This repo now contains UFS-Aerosols, the aerosol component based on NASA's GOCART. Since we are using a single repository for both S2S and coupled aerosols, the -a option was removed from checkout.sh and added to build_all.sh. This option is passed to build_ufs_coupled.sh and partial_build.sh, which have been streamlined and now accept both short and long options: build_ufs_coupled.sh: -a, --aerosols -c, --coupled partial_build.sh: -a, --aerosols -c, --coupled -h, --help -v, --verbose
…d FV3 namelist parser Refs: NOAA-EMC#230
- removed CROW FirstTrue check when building config.nsst from nsst.yaml - replace nst_spinup_logic with NST_SPINUP variable that is parsed from variable dictionary - nst_spinup_logic not needed currently Refs: NOAA-EMC#230
- set NST_MODEL=2 (turns it on) - add additional NSST namelist variables: NST_SPINUP, NST_RESV, ZSEA1, ZSEA2 - set NST_SPINUP=1 (NSST will spinup) - change CCPP_SUITE from FV3_GFS_v16_coupled to FV3_GFS_v16_couplednsst Refs: NOAA-EMC#230
Adds the eps_imesh setting to the nems.configure files when CICE is on. Refs: NOAA-EMC#324, NOAA-EMC#333
The modulefile in UFS doesn't load some modules needed for the forecast script at runtime, notably produtil, which provides err_chk. The module for the forecast job is changed back to module_base for the time being. Refs: NOAA-EMC#336
This PR is being rolled into PR #359 after all, as I needed the changes to the checkout/build for the updated UFS. |
* Reomve legacy capability of reading model input in grib1 and UPP's dependency on gfsio lib. * Update CMakeLists.txt to remove comment lines.
This PR replaces the original coupled atmosphere-aerosols system (FV3-GOCART) with the latest revision of the ufs-weather-model including prognostic aerosols. The model's code is temporarily retrieved from a forked repository as the corresponding updates are being integrated in the authoritative ufs-weather-model repository. Aerosol test cases are also updated, and the original
cplchem
flag is reused to enable coupling with the aerosol component, removing the additionalcplgocart
flag.The included changes partially address #324, updating FV3 namelist settings to run the latest FV3 revision with the
FV3_GFS_v16
CCPP suite used with the coupled UFS Aerosols component.Commit d16630f also fixes a bug reported in #337.
Closes: #336, #337
Refs: #324