driver: icedrv_history: fix start time for NetCDF history output

[davidclemenssewall]

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium, please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

• [X ] Short (1 sentence) summary of your PR: The start time for netCDF output was hardcoded to be at 00:00:00 and there was a bug in icedrv_calendar such that idate0 was fixed at Jan. 1. Both bugs are fixed.
• [X ] Developer(s): David Clemens-Sewall
• [X ] Suggest PR reviewers from list in the column to the right.
• Tony Craig, Dave Bailey
• [X ] Please copy the PR test results link or provide a summary of testing completed below. Icepack base_suite 128/128 passed. CICE quick_suite 16/16 passed.
• How much do the PR code changes differ from the unmodified code?
  • [X ] bit for bit (except for origin of time dimension in netcdf output)
  • [ ] different at roundoff level
  • [ ] more substantial
• Does this PR create or have dependencies on CICE or any other models?
  • [ ] Yes
  • [X ] No
• Does this PR add any new test cases?
  • [ ] Yes
  • [X ] No
• Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)
  • [ ] Yes
  • [X ] No, does the documentation need to be updated at a later time?
    • [ ] Yes
    • [X ] No
• [ ] Please provide any additional information or relevant details below:

[apcraig]

@davidclemenssewall, I'm curious about this fix. What was the problem exactly? Was the netcdf time axis off when starting at some time other than 00:00:00? If that's the case, I think a clearer fix would be to leave the netcdf time axis as before, relative to 00:00:00, but just update the calculation of the time values on the netcdf time axis. Does that make sense?

[davidclemenssewall]

@apcraig There were two issues here and the symptom of both of them was that NetCDF history output was always labelled as starting at YYYY-01-01 01:00:00 regardless of what value was set for istep0. For example, if we set istep0 = 4320, then the first time step should be YYYY-06-01 01:00:00. However, the netcdf history file created would still indicate that it started Jan. 01. If we look at the actual data values in that file it appears that it is correctly starting on Jun. 30 (e.g., fsw is not zero).

I've implemented the time value calculation as you've suggested (see most recent commit). Note that with this change the idate0 variable is actually no longer used anywhere in the code and it was kind of misleading anyways so I've removed it. Also, for the netCDF history files the year was restricted to be a four digit number, which didn't seem to have any purpose (netCDF and cftime don't have this restriction). So I removed that restriction as well.

[apcraig]

@davidclemenssewall, thanks for the latest updates. These kind of changes always scare me. The calendar is very delicate in general. In fact, the CICE time manager was refactored a while ago, but those changes were not pushed back into the icepack driver. How much testing have you done? I will also run a full test suite just to make sure everything still runs fine.

[apcraig]

Full test suite on cheyenne passes bit-for-bit, https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#e0560169c439a9aaf3a3f2024e42f7d6a009fd1f.

[davidclemenssewall]

@apcraig I ran the Icepack base_suite and the CICE quick_suite (both of which all passed). I also did a number of sanity checks of what time range output should cover with different values of istep0 and npt. The changes in icedrv_history only impact the history output, so there shouldn't be any risk of them impacting the rest of the calendar.

In icedrv_calendar, the only change is to cause this line (call sec2time(nyr,month,mday,basis_seconds+time)) to actually have the effect that the comment above it says that it should. That is, before changing 'sec' to 'time' in the final argument that line always set month and mday to both be 1, regardless of the value of istep0. I don't think fixing that could damage the calendar, but I'm only just getting to know this code base so I'm not sure what to check.

[apcraig]

I know the test suite runs fine. Just to confirm that you tested some cases where the start date/istep/npt were offset and you've confirmed this fix works fine in those cases too?

[davidclemenssewall]

I tried a couple different combinations of istep0 and npt and confirmed that the history files started at the right times and had the correct number of rows. I also spot checked some of the forcing variables (e.g., flw) to make sure that they looked the same at the same time of year independent of when istep0 was.

[apcraig]

Thanks @davidclemenssewall.

[apcraig]

The CICE time manager was rewritten a while ago, so it's different now. I think any fixes like this in the Icepack driver are probably OK now as then are no longer connected to coupling.