Make meaning of "prev" consistent between various clm_time_manager routines
- Dominant language
- Fortran
- Stars
- 352
- Forks
- 361
- Avg merge
- 2d 21h
- Merged PRs (30d)
- 7
Description
Emerging from https://github.com/ESCOMP/CTSM/pull/3017#issuecomment-2744267669:
Some routines in `clm_time_manager.F90` query some value for the `prev` time – i.e., the time at the start of the time step. There are two different ways this is done:
(1) Some use a `-dtime` offset to a call: `get_prev_calday`, `get_prev_days_per_year` and `get_prev_yearfrac`
(2) Some use the ESMF routine `ESMF_ClockGet` with the `prevTime` argument to get the previous time: `get_prev_date` and `get_prev_time`
I dug into the implementation of (2) (in the course of working on #3017 ). It turns out that, in the esmf wrf time manager, the implementation of `ESMF_ClockGet`'s `prevTime` argument matched the implementation in (1). However, with the real ESMF library, the implementation of `prevTime` uses the stored value of the previous `currTime` from before the most recent call to `ESMF_ClockAdvance`.
I think these two will usually be consistent, and I *think* they'll always be consistent in CTSM for now, since dtime is constant. So as far as I can tell, there aren't currently any problems. But they can theoretically differ – for example, if there is a variable clock time step, or if the clock has its time changed via a set call, rather than only ever changing via an advance of one time step. In any case, though, it's confusing to have these different behaviors, and it could cause subtle problems in the future if there were ever a variable time step, or a jump in time.
To avoid subtle weirdness in the future, and to reduce confusion now, I recommend choosing one or the other of these two. I feel like (1) is more straightforward and less prone to subtle issues under unusual use cases, so I'd recommend changing `get_prev_date` and `get_prev_time` to have implementations more like (1) (though note that this will require adding an `offset` argument to `get_curr_time`).
Contributor guide
Assessment
This issue has not been assessed yet.