NCAR / DART

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

bug: obs_window_days and obs_window_seconds are no-ops #520

Open nancycollins opened 1 year ago

nancycollins commented 1 year ago

:bug:

Describe the bug

Somewhere in the translation of the documentation from html to rst, the info about perfect_model_obs and filter ignoring the obs_window_days and obs_window_seconds namelist items got lost. the docs should be updated to say these are not yet functional, and the code should warn the user if they try to use it.

(eventually i think they could be implemented and be useful, but that's a slightly larger task needing a lot of testing.)

it would be quick to at least flag that these aren't implemented yet. the default values of these namelist items are -1. in the filter_mod, in filter_set_window_time(), if the values aren't both -1, it could print a warning or error saying this option is not yet supported. that would at least prevent someone from using them and thinking they were working.

to reproduce, set obs_window_days and obs_window_seconds to something other than -1. nothing changes in which obs are assimilated.

another thing i'd fix - the filter_set_window_time() routine only tests obs_window_days to see if it is >=0. it should also look at obs_window_seconds, i'd think. both default to -1 but someone could leave days as -1 and try only setting seconds. that should be a real error. (same for any time pair of days/seconds.)

i'd be willing to put in code to at least catch this and print a warning or error (your vote here?) if you think that's a good thing to do. let me know?

Error Message

no errors are printed - the settings are silently ignored.

Which model(s) are you working with?

any model.

Screenshots

If applicable, add screenshots to help explain your problem.

Version of DART

Which version of DART are you using? You can find the version using git describe --tags

Have you modified the DART code?

Yes/No
If your code changes are available on GitHub, please provide the repository.

Build information

Please describe:

  1. The machine you are running on (e.g. windows laptop, NCAR supercomputer Cheyenne).
  2. The compiler you are using (e.g. gnu, intel).
hkershaw-brown commented 1 year ago

For reference, here are the html versions vs the rst versions

Perfect model obs html has the "not used" https://github.com/NCAR/DART/blob/96f4dc88b0f3e00e1de672a47cb68daf069a98b5/assimilation_code/programs/perfect_model_obs/perfect_model_obs.html#L222-L226 view in html: http://htmlpreview.github.io/?https://github.com/NCAR/DART/blob/96f4dc88b0f3e00e1de672a47cb68daf069a98b5/assimilation_code/programs/perfect_model_obs/perfect_model_obs.html#L222-L226

which is in the current dart docs: https://docs.dart.ucar.edu/en/latest/assimilation_code/programs/perfect_model_obs/perfect_model_obs.html

Filter mod html says "Assimilation window days; defaults to model timestep size" https://github.com/NCAR/DART/blob/96f4dc88b0f3e00e1de672a47cb68daf069a98b5/assimilation_code/modules/assimilation/filter_mod.html#L380-L383

current dart docs follow this: https://docs.dart.ucar.edu/en/latest/assimilation_code/modules/assimilation/filter_mod.html

view html for html docs: https://htmlpreview.github.io/?https://github.com/NCAR/DART/blob/96f4dc88b0f3e00e1de672a47cb68daf069a98b5/assimilation_code/modules/assimilation/filter_mod.html#Inflation

braczka commented 1 year ago

@nancycollins thanks for looking at this. I would be in favor, as you say, to print a warning message if either obs_window_days or obs_window_seconds are set to values other than -1. Also the current dart documentation for these namelist settings in either perfect_model_obs or filter don't indicate (to me at least, maybe that's what 'reserved for future use' means?) that they are non functional. In the short term maybe make that more explicit in the DART docs.

nancycollins commented 1 year ago

ok - i will add some code to print warning messages if these are set to something other than -1 -- or i can make it a fatal error just to be safe? and i will change the docs to say they are only reserved for future use and currently should not be used.