Skip to content
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 scripts to use the GW-RT global namelist #3017

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

dpsarmie
Copy link

Description

This PR changes the ways the input.nml file is created. Instead of creating a namelist, the variables that are set locally will now use the UFS global_control.nml.IN template and the atparse function to generate the namelist.

This should allow the GW branch to more easily test changes in the UFS regression test framework. It will also allow GW to more easily introduce UFS changes into GW.

The functionality in parsing_namelists_FV3.sh is mostly unchanged. Since variables are still saved as local, the parsing of the template will occur in this script. Sections in the namelist that were previously not generated (stoch physics options for example) will be set and parsed since the regression test template has these placeholders active. As options are turned off and on, ! will be used in the namelist to comment out sections that are not used.

Previous PR #2733 was closed since my old branch was stale and a new branch was created.

Unresolved issues and input needed from GW group before final code review

First of all, global_control.nml.IN has been added in the root directory of global_workflow. This will not be in the final commit. This is a place holder until the pathing to the UFS template is finalized. A variable with the final path will also be needed to replace the hardcoded path in parsing_namelists_FV3.sh.

There are some variables that were not set in the original scripts that have been added. These were set to the UFS defaults.
The tests I've conducted were to generate the namelists but more tests should be done to make sure that outputs have not changed.

Resolves #2731
Resolves ufs-community/ufs-weather-model#1664

Outstanding dependencies:
ufs-community/ufs-weather-model#2425

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

How has this been tested?

Tested namelist generation with C48ATM and C48S2S configurations. Further testing will need to be done to ensure intended funcitonality.

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • I have made corresponding changes to the system documentation if necessary

@WalterKolczynski-NOAA
Copy link
Contributor

Not a fan of the hiding, at least not how it is implemented here. All the variables are still set by the parsing script, it makes the template more difficult to read, and doesn't even remove it from the rendered namelist, just comments the lines out. Given that, I'd rather just leave all the settings in the namelist with the values that turn them off.

@dpsarmie
Copy link
Author

Hey Walter,
Agreed, this wasn't my first choice but I didn't want to completely overhaul the scripts on the first go.
We could think about also integrating the default_vars.sh script into this workflow, but I didn't know how you all would feel about saving the environmental variables globally vs locally. If it is allowed, then you could in theory run the default_vars.sh script to set any variables that aren't defined by the user's previous scripts before populating the namelist. That should cut down on the amount of bulk in the parsing_fv3 script.

Splitting up the sections into different scripts could also work, but you run into the issue of not being able to easily pass variables between scripts if they are saved as locals (correct me if I'm wrong here).

Also open to your suggestions on how to best make this fit within the GW system.

@dpsarmie
Copy link
Author

Here is a doc with the differences between variable values for GW and RT for a C48 atm only case: GW-RT differences

We're leaning towards just changing the RT baselines to match what GW has, but we can discuss this if needed.

local NPZP=${LEVS} #levp
local GFS_DWINDS=${gfs_dwinds}

local FHZERO=${FHZER}

Check notice

Code scanning / shellcheck

Possible misspelling: FHZER may not be assigned. Did you mean FHZERO? Note

Possible misspelling: FHZER may not be assigned. Did you mean FHZERO?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old script saved this as FHZER (ush/parsing_namelists_FV3.sh L196). Kept the convention for now.

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