NOAA-GFDL / pace

Re-write of FV3GFS weather/climate model in Python
Apache License 2.0
12 stars 11 forks source link

Read in ak, bk coefficients #36

Closed mlee03 closed 7 months ago

mlee03 commented 10 months ago

Purpose

The ak and bk coefficients required to compute the pressure levels are hard-coded in the eta module for km=71, km=72, km=91, and km=139 number of vertical levels. This limits the number of vertical levels a user can choose to run models. By allowing the users to provide the ak and bk values in a simple ASCII file, the users have the freedom to specify any number of vertical levels.

Code changes:

In this PR,

  1. The hard-coded ak and bk array values have been removed. Instead, these values are read in, for example, from the input/eta79.txt file where the input directory is expected to reside in the directory of the main program and 79 represents the value of km. Checks were put in place to ensure that

    • the size of the ak and bk arrays are equal to the specified value of km
    • the resulting values of eta and eta_vare monotonically increasing
  2. Existing unit tests have been modified to read in the coefficients from an eta file and new unit tests were created to test the modified eta module. These tests were modified under the assumption that in the future, all input files will reside in one main input directory where the input directory resides in the directory of the main program. This assumption required creating input directories to all the possible testing directories in tests.

  3. Lastly, the functions vertical_coordinate and compute_eta, originally located in the baroclinic_jablonowski_williamson module have been moved to a newly created util module in the pace/util/grid directory. This change was made because it seemed more fitting for the abovementioned functions to reside with other grid-related functions.

Requirements changes:

Checklist

Before submitting this PR, please make sure:

Additionally, if this PR contains code authored by new contributors:

lharris4 commented 10 months ago

Hi @mlee03 . Glad that you could clean up the predefined vertical levels, which is getting to be a real mess. I agree that the best place for creating the vertical levels is in the preprocessor.

One concern (not related to this commit) is that there is no foolproof way to automatically generate a good set of ak/bk levels, and when creating an entirely novel set of levels some degree of hand-tuning is necessary to avoid stability problems. Xi Chen had developed a tool which can help, and interpolating from predefined level sets is usually pretty good, but some user discretion is needed when making new level sets.

NB: In the future when I have broader comments like this, is there somewhere more appropriate to bring them up than in the comments of a semi-related PR? (Adding @bensonr @oelbert for their opinion.) Thanks.

FlorianDeconinck commented 10 months ago

@lharris4: There's no way to auto-generate stable levels, but is there a way to check that they are unstable? We could eventually code that in.

lharris4 commented 10 months ago

@FlorianDeconinck there is no a priori way to ensure stability but typically we can examine the differences in delp and delz in a variety of situations, which is what Xi's tool does. We could run tests to ensure that the differences do not exceed some tolerance. This might be a good way forward; furthermore smoothing can be applied to improve the smoothness and stability of the selected coordinate.

FlorianDeconinck commented 10 months ago

Logged for posterity: #38

On the other topic, though obviously I am but a stranger here, I shall remind that there's a "Discussion" system on Github, which is a wiki-liky structure and could accommodate ongoing discussions and/or thinking on feature sets.

lharris4 commented 10 months ago

@FlorianDeconinck thanks for logging that. Also the discussions could potentially be a great resource. There are a precious few discussions on the FV3 repo and I would like to see this functionality used more.

mlee03 commented 8 months ago

Hello, sorry for the super long delay, the PR is ready for another review! I should note that there are rather many import statement changes. This was due to circular import module dependency issue between a module in fv3/initialization and in util/grid which lint did not like.

fmalatino commented 7 months ago

Please add your name to CONTRIBUTORS.md

mlee03 commented 7 months ago

@oelbert @bensonr ready for another round of reviews :crossed_fingers:

bensonr commented 7 months ago

@mlee03 - can you or one of the other reviewers discuss the necessity of the empty init.py file in the tests/main/grid/ directory

mlee03 commented 7 months ago

@bensonr, good point, to my limited understanding, I don't think it's needed here since we're not creating a package and the test is not being referenced by any modules. I think I created it here because there was one is the tests/main/driver directory :grin: :grin: :grin: but I will remove it

bensonr commented 7 months ago

@mlee03 - is there a procedure for creating the ak/bk netcdf files and is it documented anywhere?

mlee03 commented 7 months ago

I took the ak/bk coefficients from what was coded in the original version of the eta module and created a python script to generate the netcdf files. I can document how to use the python netcdf module or the xarray module to create these files. Where should it be documented?

mlee03 commented 7 months ago

Or a Jupyter notebook?