NCAR / DART

Data Assimilation Research Testbed
https://dart.ucar.edu/
Apache License 2.0
196 stars 145 forks source link

get_dist for VERTISSURFACE #757

Open hkershaw-brown opened 4 weeks ago

hkershaw-brown commented 4 weeks ago

Moved from #756

location_mod get_dist "! Vert distance can only be done for like vertical locations units" why can you not calculate vertical distance between two points one with VERTISSURFACE(m) and one with VERTISHEIGHT(m)?

https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/assimilation_code/location/threed_sphere/location_mod.f90#L507-L509

Originally posted by @hkershaw-brown in https://github.com/NCAR/DART/issues/756#issuecomment-2435532825

hkershaw-brown commented 4 weeks ago

copied comments over:

nancycollins commented 1 hour ago it's because the vertical conversion is almost always model-dependent. the location mod expects the model mod to do the conversion before calling get_dist.

@nancycollins Member nancycollins commented 1 hour ago p.s. in this case the location mod can't know the height of the surface to do the calculation.

@hkershaw-brown Member Author hkershaw-brown commented 35 minutes ago VERTISSURFACE is defined as surface value (value is surface elevation in m) so no conversion needed between VERTISSURFACE and VERTISHEIGHT

https://docs.dart.ucar.edu/en/latest/guide/creating-obs-seq-real.html#observation-locations

hkershaw-brown commented 4 weeks ago

this text in the docs:

https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/guide/creating-obs-seq-real.rst?plain=1#L333-L337

Is this true for VERTISSURFACE? That you would want want anything surface to impact the whole column? This is not what the code is doing.

https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/assimilation_code/location/threed_sphere/location_mod.f90#L501-L520

hkershaw-brown commented 4 weeks ago

mpas example https://github.com/hkershaw-brown/DART/tree/mpas_surf_height

nancycollins commented 4 weeks ago

i don't believe that DART itself requires that the vertical value for VERTISSURFACE be in meters. i think it's ignored. WRF and then MPAS were then free to require that observations set the value to be the elevation of the observing station so they could do rejections of obs at too different elevations. you might want to grep the code because i may be confused, but i don't think we require that the vertical value be anything when the vertical type is VERTISSURFACE. that doesn't mean we can't change it if that's useful.

hkershaw-brown commented 3 weeks ago

so what makes you say "i don't believe that DART itself requires that the vertical value for VERTISSURFACE be in meters'? Was the design originally no value, and people 'hacked' a use for it to reject surface observations?

Looking at the code and docs there are a bunch of places where VERTISSURFACE is m is assumed (or according to comments, defined).

https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/assimilation_code/location/threed_sphere/location_mod.f90#L63

https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/assimilation_code/location/threed_sphere/location_mod.f90#L896-L902

https://docs.dart.ucar.edu/en/latest/guide/vertical-conversion.html

https://github.com/NCAR/DART/blob/e2188646b97573f198c66f7ea63482ebc34afa11/assimilation_code/programs/obs_selection/obs_selection.f90#L100-L106

It seems prone to bugs to have a converter put out VERTISURFACE where the vertical could be in pressure, height, no value, etc and the model can interpret that number differently.

nancycollins commented 3 weeks ago

how i remember it is that originally if a location was marked as VERTISSURFACE, the actual vertical value was undefined (meaning unused, not specified as needing to be set, etc). then the wrf folks decided to use it for their own purposes since it was unused. (this was 10-15 years ago.) at that point i probably should have made it part of the spec that the value needed to be vertical in meters, or something. but it was model dependent so i just left it alone. after that point, since i knew it was being used that way, i'm guessing some code migrated into the mainline dart code that assumed meters, without it ever being part of a spec. it wasn't good, but that's the history as i remember it. i agree it's bug prone for obs.

hkershaw-brown commented 3 weeks ago

ok thanks. "observation location is model dependent." 🙃