NCAR / DART

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

loops over calendar types are unneeded #653

Open kdraeder opened 3 months ago

kdraeder commented 3 months ago

Describe the bug

The time_manager subroutines set_calendar_type_string and get_calendar_string give the right answers, but the loops over the calendar types (0,...,max_type) are unneeded and can be confusing.

  1. List the steps someone needs to take to reproduce the bug.
    To see the bug, put print statements in the loops that show the loop index and the input and output variables. Build and run a program that handles calendars; filter, model_mod_check, ...
  2. What was the expected outcome? When a loop finds the answer it needs, it should exit. Or it should do a different test during each iteration.
  3. What actually happened?
    In set_calendar_type_string the loop exits after the 0th iteration if it matches a calendar, or it tries to match the same cstring 6 more times before exiting. In get_calendar_string there's a similar loop over calendar types, where the outcome is known after the 0th iteration, but it goes on to do the same test 6 more times.

Error Message

See your messages from the suggestion above here.

Which model(s) are you working with?

model_mod_check

Screenshots

  get_calendar_string  : calendar_type =            3
  set_calendar_string  : in types loop i, mystring =            0 GREGORIAN    [the correct string is found on the 0th)
  set_calendar_string  : in types loop i, mystring =            1 GREGORIAN    [ 6 more iterations find the same]
  set_calendar_string  : in types loop i, mystring =            2 GREGORIAN
  set_calendar_string  : in types loop i, mystring =            3 GREGORIAN
  set_calendar_string  : in types loop i, mystring =            4 GREGORIAN
  set_calendar_string  : in types loop i, mystring =            5 GREGORIAN
  set_calendar_string  : in types loop i, mystring =            6 GREGORIAN

Version of DART

v9.12.refactored-1264-g38f4139ba

Have you modified the DART code?

Only to print messages. I committed those to branch calendar_loops.

Build information

Please describe:

  1. The machine you are running on Mac laptop, but this happens on any machine.
  2. The compiler you are using (e.g. gnu, intel).
    gfortran, but this will happen with any compiler (unless it recognizes a spurious loop)

The intention may have been to have an array of calendar_types(0:max_type), then use variable calendar_type to access an element of it without the explicit if-blocks, but somehow that was not implemented, or was partially removed. Removal of the loops would be a simple fix. Implementation of the calendar_types array would be more involved, but would probably result in smaller or more elegent code. That might be an opportunity to make NO_CALENDAR = 0 and let actual calendars have 1,...,max_type

hkershaw-brown commented 3 months ago

just a heads up for anyone looking at the branch calendar_loops

https://github.com/NCAR/DART/compare/calendar_loops...main calendar_loops is branched before the quantile_methods release.

kdraeder commented 3 months ago

I did not look through time_manager_mod for more examples of these extra loops, or suboptimal use of calendar types.