Open hkershaw-brown opened 1 month ago
hkershaw-brown
commented
1 month ago In compute_u_with_rbf there is a loop around ens_size, for get_reconstruct but uval(1:ens_size) is set to set to the single value ureconstructzonal/ureconstructmeridional so you are getting the whole ensemble set each step of the loop
whole ensemble == last value from the loop.
hkershaw-brown
commented
1 month ago todo can be different across the ensemble
hkershaw-brown
commented
4 weeks ago hkershaw-brown
commented
4 weeks ago find_vert_indices is a wrap around find_vert_level. Remove https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/model_mod.f90#L6030
hkershaw-brown
commented
4 weeks ago convert_vert_distrib_state (state) convert_vert_distrib (obs)
super repetitive & according to this comment state is calling the interpolation anyway: https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/model_mod.f90#L4968-L4972
hkershaw-brown
commented
4 weeks ago If rbf is depricated 11 years ago, possibly it is time to remove the code?
https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/model_mod.f90#L119 note see comment on incorrect rbf calulcaiton.
hkershaw-brown
commented
3 weeks ago Shells scripts are out of date, user complaints about cycling. Lots of csh (poor).
Documentation is poor, also points people to this MPAS page which sends people to none existent documentation (https://svn-dares-dart.cgd.ucar.edu/)
hkershaw-brown
commented
3 weeks ago Note, beware of whatever the regional stuff is doing. Seems like some bizarre choices here.
hkershaw-brown
commented
3 weeks ago update_bc.f90 is calling use direct_netcdf_mod,only : read_variables directly. 🙃
hkershaw-brown
commented
3 weeks ago why is this compile time, rather than runtime: https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/model_mod.f90#L238-L243
hkershaw-brown
commented
3 weeks ago why is this compile time, rather than runtime:
oh add_static_data_to_diags is not used and there is also a compile time (namelist) option write_grid_to_diag_file
hkershaw-brown
commented
3 weeks ago ! FIXME: it may be desirable to read in xCell(:), yCell(:), zCell(:)
! to keep from having to compute them on demand, especially since we
! have converted the radian lat/lon of the cell centers into degrees.
! we have to convert back, then take a few sin and cos to get xyz.
! time/space/accuracy tradeoff here.currently unused (11 years). Not needed for regional? Or should be used? https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/model_mod.f90#L477-L480
hkershaw-brown
commented
3 weeks ago This data on edge logic seems overcomplicated
variable dimension tells you edge vs cell, .e.g
float u(Time, nEdges, nVertLevels) ;
u:units = "m s^{-1}" ;
u:long_name = "Horizontal normal velocity at edges" ;
float qs(Time, nCells, nVertLevels) ;
qs:units = "kg kg^{-1}" ;
qs:long_name = "Snow mixing ratio" ;
"causes mpas namelists to be read" - does not appear to be true
hkershaw-brown
commented
3 weeks ago This comment, not sure what it is getting at:
hkershaw-brown
commented
3 weeks ago boundary domain variables do not get bounds for clamping:
hkershaw-brown
commented
3 weeks ago query_vert_localization_coord not used & not requried.
hkershaw-brown
commented
3 weeks ago get_init_template_filename
called but value unused in update_bc ? https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/update_bc.f90#L105
hkershaw-brown
commented
3 weeks ago hkershaw-brown
commented
3 weeks ago serial loop around files for update bc: https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/update_bc.f90#L122
hkershaw-brown
commented
3 weeks ago hkershaw-brown
commented
3 weeks ago is_global_grid
Only used by mpas_dart_obs_preprocess, then global does not appear to be used
https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/mpas_dart_obs_preprocess.f90#L1013
hkershaw-brown
commented
3 weeks ago Hardcoded dom_id in update_mpas_states:
hkershaw-brown
commented
3 weeks ago serial loop around files for rupdate_mpas_states
hkershaw-brown
commented
3 weeks ago progvar is removed and replaced with state structure. Notes on done/not done routines
I'm concerned about update_bc and update_mpas_states hard coding domains and using read_variables directory. Also,
memory wise - reading the whole state in
performance wise, looping around each restart file in update_mpas_states
hkershaw-brown
commented
3 weeks ago todo docs:
hkershaw-brown
commented
3 weeks ago oncenters is not binary (3 choices)
find_vert_indices -> find_vert_level( ..., oncenters=.true., ...)
the logic for oncenters is a bit weird to be passing all the way down the call tree.
Here it is hardcoded .true. for pressure otherwise .false. https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/model_mod.f90#L6136-L6157
but oncenters ignored for pressure: https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/models/mpas_atm/model_mod.f90#L5374-L5406
oncenters(/faces/edges) only relevant for HEIGHT? oncenters/faces/edges Can be queried from the obs_kind.
see
Unresolved issues:
Surface elevation check
! Reject obs if the station height is far way from the model terrain.but fwd operators (rttov) need these surface values see discussion https://github.com/NCAR/DART/discussions/332
Code:
branch (be aware I may force push to this) https://github.com/NCAR/DART/tree/mpas-refactor
Spec (note 7 v 8) https://docs.google.com/document/d/1wD4MKM5PquOUbxK3YxIjcbIaFThJfa318heDn_bFUbs/edit