Generalized subroutine to read the state variables from the namelist (get_state_variables) #783
Conversation
…nto a table and verify its contents; putting it in the default_model_mod
Updating the generic routine by including the type state_vars_type and storing the contents of the nml entry here Including a conditional use_clamping that tells whether or not the model's nml entry includes clamping values; executing a separate do loop over the rows if true to allow for reading in these values
its functionality. Adding checks against known values with fortran-test-anything
…riables: clamp and noclamp
…ing 2 distinct input.nml files
…del_mod This model is the perfect example of how we can have separate calls to get_state_variables for each file domain instead of having the domain included in the state_variables nml item.
|
@mjs2369 Do you have restart files so you can test the changed model_mods? |
There was a problem hiding this comment.
Hi Marlee,
Good work finding all the model_mods routines that can be switched out by this.
I've put some comments in. The thing to sort out first, is the get_state_variables routine.
- Does the overloading and having to set clamping = .true. make. sense? '
- Can you keep state_vars type private, so the model_mod does not access the internals (see the comments on add_domain)?
The 3d template model_mod needs updating to use the new routines, since this is what is used when the 'new_model.sh' script is run.
Documentation for routine also needs adding, maybe 'adding a new model' is the best place for this, I'm not sure.
I think the tests should comprehensively test the whole table read - if we are running the test, should test all the values.
Only for cam-fv and pop - do you want me to test that all models are bitwise? |
If you're changing the model_mod then yes, check that the new code does the expected. |
- Make the name of the function parse_variables - Remove interface to create two distinct functions: parse_variables and parse_variables_clamp - Remove use_clamping and max_state_vars arguments - Update the calls to the function in all edited model_mods
|
this is cool Marlee. |
Co-authored-by: Helen Kershaw <[email protected]>
|
Hi Marlee, it will be next week before I look at this, I'm not super worried about it, but it is a lot of model_mod changes to drop on a Friday! Have a good weekend |
|
as in release on a Friday 'drop it like its hot' |
| call ok(state_vars%qtys(3) == QTY_U_CURRENT_COMPONENT) | ||
| call ok(state_vars%updates(1) .eqv. .true.) | ||
| call ok(state_vars%updates(2) .eqv. .true.) | ||
| call ok(state_vars%updates(3) .eqv. .true.) |
There was a problem hiding this comment.
It would be good to test that state_vars%clamp_values is not allocated, since that is what your add_domain call is using to set clamping or not.
…_mod.f90 Co-authored-by: Helen Kershaw <[email protected]>
|
|
||
| domain_id = add_domain(model_shape_file, nvars, & | ||
| var_names, quantity_list, clamp_vals, update_list ) | ||
| domain_id = add_domain(model_shape_file, parse_variables_clamp(mitgcm_variables)) |
There was a problem hiding this comment.
I don't think you can replace the routine for MITgcm_ocean because the original code adjusts the clamping values if you are doing a log transform for BGC variables:
DART/models/MITgcm_ocean/model_mod.f90
Lines 2113 to 2123 in f2deaa2
Co-authored-by: Helen Kershaw <[email protected]>
| update_list = update_var_list(1:nfields)) | ||
| ! parse_variables converts the character table that was read in from | ||
| ! model_nml:model_state_variables and returns a state_var_type that | ||
| ! can be passed to add_domain |
There was a problem hiding this comment.
This model_mod is missing the logic to use the default state variables if no model_state_variables = ' '
before calling add_domain
Lines 2774 to 2778 in f2deaa2
if model_state_variables = ' '
call use_default_state_variables( state_variables )
'blah blah using default state variables'
endif
domain_id = add_domain('pop.r.nc', parse_variables(model_state_variables))
| ! parse_variables converts the character table that was read in from | ||
| ! model_nml:model_state_variables and returns a state_var_type that can be | ||
| ! passed to add_domain | ||
| domain_id = add_domain('cice.r.nc', parse_variables(model_state_variables)) |
There was a problem hiding this comment.
Missing the logic for model_state_variables = ' '
DART/models/cice/model_mod.f90
Lines 2618 to 2622 in f2deaa2
| @@ -2684,7 +2530,7 @@ subroutine use_default_state_variables( state_variables ) | |||
| character(len=*), intent(inout) :: state_variables(:) | |||
|
|
|||
| ! strings must all be the same length for the gnu compiler | |||
| state_variables( 1:5*num_state_table_columns ) = & | |||
| state_variables( 1:5*3 ) = & | |||
There was a problem hiding this comment.
| state_variables( 1:5*3 ) = & | |
| state_variables( 1:15 ) = & |
There was a problem hiding this comment.
Hi Marlee, looks good.
I checked the template/threed_model_mod.f90 & threed_input.nml with new_model.sh
There are 3 model_mods that do need changing:
POP, CICE to have default state variable tables if none is specified in the namelist
MITgcm I don't think you can replace because the clamping values depend on whether a log transform is applied to BGC variables
Cheers,
Helen
Thanks for all this Helen. I'll make these changes and let you know when it's ready for merging |
Description:
There are many variations of the subroutine to read the state variables from the namelist in various model_mods. Previously, a new variation of this subroutine would need to be written every time a new model is added to DART.
This PR creates an interface that allows for the use of one of the two generalized subroutines that can read the state variables from a namelist:
get_state_variables_no_clamp reads netcdf variable name, dart qty, and update values from the nml
get_state_variables_clamp reads netcdf variable name, dart qty, clamp_values, and update values from the nml
Alternate versions for this subroutine were replaced with get_state_variables in the models MOM6, noah, wrf_hydro, MITgcm_ocean, aether_lat-lon, cam-fv, cam-se, POP, and cice
New dev test test_get_state_variables added
Fixes issue
Fixes #448
Types of changes
Documentation changes needed?
Tests
Compiled all models with updated model_mods
Build_everything runs as expected with all compilers
Bitwise tested with CAM-FV
New dev test runs successfully and reports pass/fails correctly
Checklist for merging
Checklist for release
Testing Datasets