Feature request: Refactor wrf model_mod and models/wrf directory #404
Comments
hkershaw-brown
changed the title
|
edit: https://github.com/NCAR/DART/tree/wrf-fresh is starting from scratch redoing the wrf model_mod |
|
wrf model_mod get_close has a test for missing_r8 should check for verti_is_undefined before doing this (or not have the missing_r8 check) |
Vertical ConvertionThere are several references to the vertical location of state being converted in get_state_meta_data. This is incorrect, get_state_meta_data does not do vertical conversion. For example: Lines 6406 to 6417 in 72e5740 get_close_obs and get_close_state are identical - they both call get_close. convert_vertical_obs calls vert_convert
|
|
To do:
|
|
note in the code about latlon project settings only valid for global domains Lines 7307 to 7316 in 9729d78 |
|
for help with the acronyms:
comments on these particular few lines: |
|
Mask variables: XLAND, QTY_SURFACE_TYPE Should you be interpolating a mask? wrf netcdf file XLAND:description = "LAND MASK (1 FOR LAND, 2 FOR WATER)" ; edit: Lines 2825 to 2843 in 9729d78 Lines 2801 to 2823 in 9729d78 |
|
I'm not sure what is not allowed here Lines 6738 to 6740 in 9729d78 There are some notes in the code about periodic_x,y and polar Line 7092 in 9729d78 It is looking like these are still wrf namelist options and not in the wrf output netcdf file: |
|
Boundary condition options: The boundary conditions periodic_x,y, polar in model_mod.f90 only apply to domain 1. I guess this is why it is a namelist option rather than part of the wrf netcdf metadata(?). There is a lot of checking for in model_mod for (id)%periodic but periodic is only allowed to be true for domain1. Lines 7114 to 7124 in 9729d78 Edit: the wrf namelist has periodic_x,y polar for each domain - namelist (max_dom) The wrf docs has
Edit: What is the difference between polar and periodic_y? no Lines 6275 to 6277 in 70e6af8 |
|
|
Edit also a set of constants in wrf model_mod: Lines 7997 to 8006 in 9729d78 Edit2: |
Needs logic to deal with obs below lowest level Needs static data to calculate rho Using two different values for gravity see #404 (comment) I'm not sure about the check for 0.0 for surface pressure. Does this happen? For model_rho_t checking speficically for MU variable name rather than qty_pressure
|
Original code is using one ensemble member see: #404 (comment)
see not on periodic_y vs. polar #404 (comment) remove mass grid comment from getCorners since this checks for U,V grid
The vertical conversion is difference for state vs obs. see #404 (comment) for note on the (no longer true) assumption in the old code that state elements have already been vertically converted in get_state_meta data.
|
…el dt Previous code would let you set assimilation_period_seconds = 0 which gave you a 0 sized assimilation window #404 (comment)
|
For convert_vertical_obs, it looks like there is a biggish difference when you change order of calculating geometric from geopotential height. Maybe this is just a bug on my end, but making a note of it here:
Then interpolate to geometric_height observation location.
Example of differences: |
…ight This is quite different from main. I'm not sure if this is a bug on my end or the order of caluculation makes a big difference. See #404 (comment) for details
…main) This is method 1 in #404 (comment) for discussion zout same value as main
|
i don't know if this is a related issue, but this reminds me that we had many heated discussions about interpolation order in a cube. i believe if everything is linear the order doesn't matter (within numerical error limits). but if the values are not - like with pressure - then it makes a difference in the results. i'll try to describe this without images. if you label the 8 corners of a cube (A,B,C,D) for the top, clockwise, and (E,F,G,H) for the bottom, also clockwise, then the two ways you can do interpolation to a point in the interior are:
we have (i think) tended to do method 2, but honestly right now i can't reconstruct the arguments for and against. it has to do with non-linear fields in the vertical near orography. |
|
Comments from Jeff and Moha from standup: For easier comparison (bitwise crutch) with existing wrf code I'm leaving in |
|
https://github.com/hkershaw-brown/DART/tree/wrf-refactor is the initial attempt, having a go at updating the wrf model_mod. It is essentially my notes about the code. I plopped it on my fork, just to have the notes. It is dead. https://github.com/NCAR/DART/tree/wrf-fresh is starting from scratch redoing the wrf model_mod.f90. I'm getting bitwise (last digit) differences in the distance calculations when doing vertical conversion. |
|
tiny test case: restarts - contains the restarts This case is really tiny, you can run on the login node. checkdiffs.sh is just for comparing netcdf files and obs_seq.final |
|
same or different qty? |
|
same or different qty? |
|
Lines 7485 to 7491 in 52e6e45 |
based on the latest 3d model_mod template * state variables added by namelist * removes wrf_static_data_for_data, uses state_stucture_mod Note for bitwise comparison with the existing wrf model_mod, using function interpolate_geometric_height #404 (comment)
The bitwise differences were due to this issue #621.
If you need to look at wrf-fresh: |
|
I don't think clamp_or_fail is used: Line 343 in 52e6e45 https://github.com/search?q=repo%3ANCAR%2FDART%20clamp_or_fail&type=code clamp_or_fail not passed to state structure DART/assimilation_code/modules/io/direct_netcdf_mod.f90 Lines 1448 to 1534 in 52e6e45 |
|
Todo Follow up on wrf 4 hybrid vertical coordinate https://github.com/nancycollins/dart_dev_archive/commit/c28a6e82f0a7775f2075555ab84ea32668676adf |
|
i've made a wrf_hybrid branch on my fork of the DART repo with the code changes for the hybrid coords in the wrf model_mod. it compiles. i have no input files to test with. |
|
Hi @braczka I'm moving your comment to a new issue, running DART with wrf 4+ |
Use case
Refactor the wrf model_mod to make it easier/possible to:
Is your feature request related to a problem?
The WRF model_mod has
wrf_static_data_for_dartwhich is replicating functionality available in state_structure_mod.Adding variable X to the state means altering the model_mod code to have a
wrf%dom(id)%type_X, rather than simply adding a line in the model_nml::wrf_state_variables.WRF has its own integer types (qtys), replicating (but not quite) the DART qty functionality.
DART/models/wrf/model_mod.f90
Lines 323 to 332 in 7ee9ed9
so you see code like this:
DART/models/wrf/model_mod.f90
Lines 889 to 890 in 7ee9ed9
WRF v3 vs. v4 - it is not clear for users what to change when moving from WRF v3 to v4. If possible, it would be good for the model_mod to be able to check the WRF version from the netcdf file.
Wider issue Model_mods which use their own structure to keep track of state variables #389. Moving control of the state vector to core dart modules allows us to have general solutions for things like compression, removing missing values (so dart core code can assume every element of the state is valid), and other cool stuff.
Describe your preferred solution
Remove the state vector information from wrf_static_data_for_dart
Describe any alternatives you have considered
I think the wrf_static_data_for_dart is a road block for core dart development, however wrf-dart is widely used and has a lot of additional user developed code in circulation. There is 8000 lines of code in for wrf model_mod (+ unknown user modifications) so the refactoring needs to be approached with this in mind.
I think it is possible to simplify the bounds checks for each grid (staggered and various flavors of periodic), but I do not think the bounds check affects development of core dart.
The text was updated successfully, but these errors were encountered: