Open billsacks opened 6 years ago
Bill Sacks < sacks > - 2011-11-22 15:44:37 -0700
A number of pieces of code won't work correctly for long time steps if leap years are allowed:
(1) dt isn't modified from one timestep to the next. This sort of dynamic step size would be needed for, e.g., 1-year time steps. This is also a problem for any time step specified in terms of the dt_option 'steps_per_year'
(2) The loop in the reduce_seconds subroutine doesn't allow for variations in seconds_in_year from one iteration to the next. I believe this is only a problem for time steps longer than one year.
Bill Sacks < sacks > - 2011-12-05 14:46:25 -0700
Nancy Norton suggests: handling leap years may be easier if we just write special purpose subroutines to handle timesteps of, say, multiples of a year (and other than that, don't allow timesteps greater than a year).
Bill Sacks < sacks > - 2011-12-08 20:21:25 -0700
(2) The loop in the reduce_seconds subroutine doesn't allow for variations in seconds_in_year from one iteration to the next. I believe this is only a problem for time steps longer than one year.
It turns out there is a larger problem with the reduce_seconds subroutine when leap years are allowed: it uses the value of seconds_in_year that was set most recently, which in the current flow through the code is NOT the seconds_in_year for the current time step. This may a problem in other places in the code, too - the determination of whether it's a leap year (and the consequent setting of seconds_in_year and other variables) is out of sync with some of the places that use the information on whether it's a leap year, so these leap year-dependent variables sometimes refer to the wrong year.
This problem shows up for time steps shorter than a year - e.g., on the order of 6 months to a year. The problem shows up in the oneyear_leap test in test/unit/time_management, which tests a 365-day time step with leap years active. For this test, the model aborts with the error, "Unhandled condition in ymd_hms: iday > days_in_month", but the real problem seems to be that the leap year variables are out of sync.
I don't think this is a problem for short time steps (e.g., order 1 day), but I'm not positive at what point it becomes a problem.
The current flow in time_manager is:
(for time step = 1, reduce_seconds called for the current time step)
reduce_seconds called for the next time step
model_date called for the current time step, which includes determining iyear and calling leap_adjust for the current time step
Thus, if I understand this right, the main reduce_seconds call - i.e., the one that operates on the values for the next time step - uses seconds_in_year from the previous time step. I would need to think more about whether it should be using seconds_in_year from the current time step or the next time step, but I know it's wrong to be using seconds_in_year from the previous time step.
Bill Sacks < sacks > - 2012-10-11 10:53:44 -0600
I noticed that the driver's build-namelist doesn't allow a leap year calendar in conjunction with NCPL_BASE_PERIOD=year or decade. Since those NCPL_BASE_PERIODS are the only way to get dt_option=steps_per_year (unless the user overrides dt_option manually), I think it's not important for us to handle leap years in conjunction with dt_option=steps_per_year.
However, it's probably still worth fixing some of the other leap year-related bugs in this bug report.
Bill Sacks < sacks > - 2016-11-10 14:10:00 -0700
In a recent discussion, Mariana suggested ditching the current time manager (based on POP, which has quite a few complexities) and using a much simpler one. I may want to do that rather than patching up the current time manager.
Bill Sacks < sacks > - 2011-11-18 11:45:16 -0700 Bugzilla Id: 1433 Bugzilla CC: erik, lipscomb, mvertens,
I am modifying the time manager to allow longer timesteps (multi-month, annual or multi-year). However, I am not fixing everything right now.
Things I have found that need to be fixed in the future:
determination of newhour in subroutine time_manager: I believe that this line:
if (ihour /= ihour_last) newhour = .true.
will not work for time steps that are multiples of a day
Also, I have not checked all of the code completely to be sure that things will work correctly for longer timesteps. In particular, I have not checked the code that determines stopping conditions: does any of this stop-related code implicitly assume timesteps shorter than about 1 month? Similarly for the time_to_do and time_to_start functions.