Bug in planting window logic (no effect given current parameters?)

[samsrabin]

Brief summary of bug

If a planting window overlaps the new year, then a crop will only ever be planted on the last day of the window. This bug has no effect for now as far as I'm aware, because no planting windows actually do overlap the new year.

General bug information

CTSM version you are using: ctsm5.1.dev054

Does this bug cause significantly incorrect results in the model's science? Not given current parameterization.

Configurations affected: Unknown

Details of bug

The "normal" (i.e., not "last-chance") planting condition for crops other than winter cereals requires, among other rules, that (lines 1928–1929):

jday >= minplantjday(ivt(p),h) .and. &
jday <= maxplantjday(ivt(p),h)

What if the planting window spans the new year? In that case, minplantjday > maxplantjday, and therefore this rule will never be satisfied. That would result in planting only ever happening in "last-chance" mode at the end of the planting window, because the only window-related condition there is that jday == maxplantjday(ivt(p),h). (Winter cereals would be similarly affected.)

A safer method would be to use a function that checks whether today's date is within the window defined by minplantjday and maxplantjday, accounting for whether minplantjday > maxplantjday. Does such a function exist?

This has probably never come up before because no planting windows actually do overlap the new year. Here is the unique set of planting windows, in format (M)MDD, from /glade/p/cesmdata/cseg/inputdata/lnd/clm2/paramdata/clm5_params.c200717.nc:	min NH	max NH	min SH	max SH
401	615	1001	1215
901	1130	301	530
501	615	1101	1215
401	531	901	1130
101	228	1015	1231
101	331	801	1031
320	415	920	1015
415	701	1015	1231

Important details of your setup / configuration so we can reproduce the bug

N/A: Bug only evident from code, not (as far as I'm aware) any actual results.

[billsacks]

@samsrabin great catch. I agree with your proposed solution. I am not aware of any current function that does what you suggest. Do you want to take on fixing this? (I don't want to punish you for finding & reporting bugs like this, but if you have the time and interest to fix this, it would be much appreciated. On the other hand, if you don't want to get side tracked with this right now, that's fine, too.)

[ekluzek]

@samsrabin if you do implement this I think you should add a

IsWithinTimeWindow

logical function in clm_time_manager.F90

The implementation would be fairly straight forward you turn the start and end dates into ESMF time instants and then do comparisons. The one catch would be if the start time is after the end time you have to reverse the logic. Unless you also account for the year advancing for that case when you could keep the logic the same.

There is a unit-tester for clm_time_manger which would make it much easier to implement and test this function for all the different types of cases.

[billsacks]

+1 to @ekluzek 's comment, with the minor edit that it should be is_within_time_window to follow our (currently inconsistent, but intended for the future) naming conventions (https://github.com/ESCOMP/CTSM/issues/835).

[samsrabin]

Resolved with #2158.