Make time axis

[alexkjames]

Addresses #330

[alexkjames]

Still to do:

*Fix all of the stuff that broke with these changes
*Update gkernel to align behavior
*Update common time to align behavior

*Test methods and make sure defaults are all the same
*Test methods to make sure everything works as advertised

[CommonClimate]

Test methods and make sure defaults are all the same
defaults should be the same for gkernel and bin. I was under the impression that, once interp() can properly handle NaNs, then we should allow users to specify finer time resolutions, in a way that would trip up the other two methods. Am I mis-remembering?

[alexkjames]

@CommonClimate You're mostly correct. However, all of the methods will be allowed to specify finer time resolutions. The defaults for bin and gkernel will be to avoid this behavior, though the user will be free to do these things if they so wish. So 'make all defaults are the same' is a bit misleading on my part, I think that was the product of friday brain fog. I meant it should be that if you pass in the same arguments to each function, you'll receive the same time axis. The value axis will ideally be the main thing that varies between each method. If this is out of line with your expectations I'm happy to discuss further, I was under the impression that one of the main reasons we were overhauling these functions was to put them on similar semantic ground

[CommonClimate]

Yes, we are in agreement ; I was just thrown off by 'make all defaults are the same' . The function will indeed be common to all regridding methods, and I was envisioning that it would be called within the API level functions bin, gkernel, interp with defaults set at the level of those functions. Once those behave as they should on realistic series, then I expect common_time will be greatly simplified.

BTW, I am thinking that instead of make_time_axis we should actually call this utility function make_even_time, because it will be even, correct? If so, that is an important keyword to put in. I suggest to set no defaults at all for this function at the utils level, so that we make it clear that it is the job of the API-level functions that call it to set appropriate defaults. However, if you see a reason to have universal defaults, let me know.

[alexkjames]

I see, currently most of the work done for each of these functions is done within the utilities, so just to be explicit we would be changing that to having the time axis step done at the API level right? Currently I have this being done within the utilities as well, though it should be straightforward to change.

[CommonClimate]

Yes, done within the utils-level function, but the setting of the step should be done at the API level, because it will vary between those regridding types (.e.g interp will use a different step style than bin). Do start and stop need to vary as well? Perhaps not, so I suggest having them set as defaults in the utils-level function, and only override them if need be.

[CommonClimate]

The key is to have all the common bits handled by the same piece of code, to minimize unwanted variation. But some variations are desirable, and should be made explicit!

[alexkjames]

Well put! One minor question that has risen with regards to how start,step and stop are interpreted is with regards to the difference between interp and bin. For interp these arguments are used to specify the time axis which will serve as the basis for interpolation. This usage is also possible for bin and gkernel, but requires "half bins" at the beginning and end of the time axis, as the bin edges must be set as the midpoints between times, but should not (in my opinion) extend beyond the start and stop times. For example, if start = 0, stop 3, and step (or bin_size) = 1, the time axis = [0,1,2,3], but the bins are [0,.5,1.5,2.5,3] (note the "half bins" at the beginning and end). The alternative is to have the "time axis" in this case just define the bin edges, in which case, using the above example, bin_edges = [0,1,2,3] and the time_axis = [.5,1.5,2.5]. I think the latter makes more sense from a contextless perspective, but it does mean that the arguments are interpreted differently between the functions. Do you have any thoughts? @khider might also have an opinion.

If my description just elicits more confusion, we can also save this discussion for the dev meeting tomorrow.

[CommonClimate]

Thanks for that. This is a clear indication that the very meaning of "time axis" varies from one application to the next, particularly this issue of edges vs midpoints. We do want to be very explicit about that, and maybe there should be two separate functions (one for bins, one for interpolation times)? Perhaps not, but worth thinking about. As long as the doc is clear, I think it's OK to have one function that does different things depending on context (edges vs midpoints), but you'll find out when implementing/testing whether that's the right approach!

[CommonClimate]

Everything looks good ; the bigger question is, does it work? Can you apply this to paleohack NB3, where I do a tour of those methods, to check that they really do work? Also, it would be good for this NB to include the option of passing a time axis, which wasn’t available before.

[CommonClimate]

NB3 looks like it works, so I'll merge.