-
Notifications
You must be signed in to change notification settings - Fork 228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible bug: PEcAn output sometime repeats first/last date/time between years or starts at -1 #2094
Comments
Here is an example of raw SIPNET output
and the end of that year
so that looks ok |
WRT SIPNET the issue is here
and I am not certain why we have the -1 there
versus
where the second would end as
|
With ED2 its a bit tricky because in python (ILAMB):
gives
and we currently provide ED2 output time such as:
|
so it seems we need to be ending in the 364. part for non leap and 365. for leap, for example
for a leap year where
results in the same issue I raised above |
For ED2 I propose we change to:
and
which would still translate to correct cal dates, eg
for leap years. @istfer @tonygardella @ashiklom thoughts? Obviously we would set the timestep dynamically based on output frequency instead of hard-coding but I think we just need to change the end date from 365 to 364 or 365 from 366 depending. |
@serbinsh I haven't seen the SIPNET issue before. I just checked a SIPNET output and it starts from day 1, could it be a version thing?
therefore:
or could it be something related to the the met2model? I think SIPNET takes the time in the sipnet.clim file into consideration. The clim files I'm using are generated a while ago, maybe latest changes affected these |
@istfer what version of sipnet are you using that gives you day as 1? I was running r136 or whatever NOT "unk" but yeah that looks like a version issue! and that could cause us issues unless we determine 0 vs 1 |
I'm using r136 as well |
@istfer OK so narrowing down, here is my input sipnet.clim file
so you can see starts with 0 and
and ends at 364. Does yours end with 365 by chance for a non leap? |
Strangely it looks like my runs did not have leap met
|
yes, for a non-leap year my clim starts with 1 and ends with 365
|
@istfer did you mean starts with 1 and ends with 365? |
ugh, yes |
@istfer what if we implement something like:
point is to be consistent with 0-364/365 depending on non/leap instead of some being 1-365/366 but then how to deal with runs less than a year.... |
arrggh that wont work if there is a leap year. |
That also won't work for shorter runs. Don't forget that we've now got 16 day SIPNET forecasts running daily at Willow Creek |
is this harder than it should be because we are inconsistent with how met days are specified to SIPNET? That is sometimes met has a 0 start and sometimes its indexed based on a start day of 1? Can we just pick one? That said, not sure how best to make this back compatible....but as it is I dont think we want to have both. I suppose we could look at the met file but then again it may not have start/end year dates if just a few weeks of met drivers. I dont know the solution right now.... |
If the issue is the difference between runk and r136 (and moving forward) then I'm fine with breaking backward compatibility with runk, dropping it from the VM, and labeling it as deprecated. There's really no difference in model guts between the two and r136 has some I/O bug fixes. |
Day of year should start with 1 to represent Jan 1. Also watch out for timezone shifts. Shifting from UTC to CST or EST or vice versa is another source of error in terms of when start/end days don't get parsed properly in input drivers or outputs. Ameriflux files are in local time, so things get shifted forward during conversions. This leads to days getting added or removed. |
@mdietze I dont think its a version issue, at least for SIPNET, it looks more like an issue of how the met drivers are specified. |
@ankurdesai issue is that 0 and 365 get parsed as the same date: #2094 (comment) |
What we need is a standard way we define time in netCDF, which includes days since, calendar type, and how the time values are specified such that we avoid the 0/365/366 issue as being read as the same date and thus duplicated. Obviously when we use leap we need to specify a leap cal and if not probably should specify a no_leap cal but also we should be consistent that values should be from 1.x to 364.x (or 365.x if leap) for all model output this is again another rabbit hole i didnt anticipate when starting this discussion...I thought perhaps it would be a fairly quick fix but it seems it actually is not |
It would seem that we actually want to first determine the posix date and then generate the DOY tvals from this.... and also that we need to start with 0, e.g.
|
...or perhaps we start to use time bounds, e.g.
|
Sounds like we've concluded that years start at 0. Are there other things to resolve here? (other than some code fixes and tests to make sure ever met2CF, met2model, and model2netcdf are doing this correctly) |
@mdietze take a look at #2095 Probably still some cleanup but I have tested on modex (command and via web) and it doesnt break PEcAn but does provide the additional "time_bounds" that is common in global ESM output and thus something programs like ILAMB looks for. Provides to start/end timepoints for each timestep which means we use time or time_bounds to define the comparison with data, etc. Where time_bounds can be used aggregate data of higher frequency into exact model output bins..etc. And as Nate suggested our outputs arent instantaneous and as such bounds provide explicit bounds for each step. either way I was hoping that we could add to the output since it doesnt impact existing functions but could add functionality in the future and provides an easier connection with ILAMB instead of have to come up with a workaround to read "time" and make assumptions in the comparison with benchmarks.....happy to explain more tomorrow |
For example, global CLMCN output
One thing I would prefer is if we could remove int variable from the output and just have the dimensional variable. this is how SIPNET is written in my PR
|
and here is CLM-FATES output
So I guess the questions are 1) are we happy with inclusion of a bounds variable, 2) if so, what shall we call it (seems for CLM its common to call it "hist_interval" of size 2) with a longname of
showing up? ....and whatever else I might be missing/forgetting. |
This issue is stale because it has been open 365 days with no activity. |
Describe the bug
When working on running some test runs of ED2 through ILAMB we discovered that, at least in my output, the last date of the year in time variable is the first date of the next years output
If we look at the times of my output from 2001-2002 for example
you can see the time is the same here as well.....this could cause us problems for processing that attempts to chain output together, such as with ILAMB currently
Also, while playing with SIPNET output I realized that in my runs the first value of the time variable is -1!
Expected behavior
I would think that we do not want the output provided in this way and instead only have the time variable include that years data/date range? For example, ending in fractional time such as:
364.895833333333, 364.916666666667, 364.9375, 364.958333333333, 364.979166666667 ;
And starting with
0.04166667
for example if hourly (i.e. SIPNET).Need to check if this is coming from met or from model2netcdf....I suspect m2n step
Machine (please complete the following information):
The text was updated successfully, but these errors were encountered: