Increase the precision when converting milliseconds to years

[gaspardpetit]

With the current implementation, a large negative millisecond timestamp will result in a relatively normal positive year (int64::min translates into year 1535). This change improves the precision for large number of days obtained from large millisecond values such that even large negative amounts of milliseconds will result in negative days and negative years.

[HowardHinnant]

The civil calendar currently models the solar system to an accuracy of about 1 day in 3300 years. That is, about every 3300 years the civil calendar gets out of sync with the seasons by about a day.

So it doesn't make a whole lot of sense to talk about the civil calendar millions of years in the past or future. It is with this in mind that year::max() == -year::min() == 32767 which fits in 16 bits. Even this 16 bit range for year is overkill, but an 8 bit range would be much too small.

Another detail is that experience with boost::date_time has taught us that developers value 4 byte dates. This library has two data structures for dates, and both of them are 4 bytes:

sys_days
year_month_day

This change would increase the sizeof both of these important data structures.

If one needs a duration with a period of days and range with greater than +/- 5 million years, that is of course possible with a custom duration as you show. But that is outside the typical use case. I don't want to burden 99% of the use cases with a greater sizeof so that 1% don't have to write a custom days duration.

The basic requirements for this civil calendar library are, if you want to avoid calendrical overflow, keep your dates in the range [-32767-01-01, 32767-12-31]. If you need to track time outside of this range, stick with the chronological types (sys_time) with durations sufficiently coarse that 64 bits will cover your desired range, or change your representation to 128 bits, or floating point (which cover more than the age of the universe at typically used precisions).

Finally, work with the knowledge that whatever period and representation you choose, there is a point at which it will overflow. It is good to be aware of that limitation.

[gaspardpetit]

Thank you for the detailed explanations (I always learn something when I play with this library :D).

I think my main concern is about robustness of code. It is not uncommon for timestamps to be represented as 64bit milliseconds. An application would normally not produce timestamps outside of 16 bit year ranges... unless it has overflowed or has been purposefully set to an invalid value to indicate error (and the code indicating the error might not be aware that it should not produce dates below -32767).

In the case I experienced, we did have a safety to check that the final date to ensure it was "reasonable", but we found out that the large negative year (hundreds of millions of years before 0) ended up nicely underflowed to the year 1535 and passed the safety checks.

The objective of this change is not so much to increase precision, but rather to handle the out of range millisecond values more gracefully. Would you have a suggestion on how this kind of error could be caught? Perhaps a is_valid method on sys_time that would be false if the time is outside the supported range?

[HowardHinnant]

I agree that overflow checking is one of the rough spots of chrono. For casual work most of the limits are far enough away that you can get away without checking. But sometimes you really do need to check, for example if the input comes from an untrusted source.

For your situation I recommend creating limits of the civil calendar but in units of sys_time<milliseconds>, and then check against those prior to converting to the civil calendar. Maybe something like:

    sys_time<milliseconds> constexpr min_time_stamp = sys_days{year::min()/1/1};
    sys_time<milliseconds> constexpr max_time_stamp = sys_days{year::max()/12/31} +
         23h + 59min + 59s + 999ms;

    sys_time<milliseconds> ts = ...;
    if (min_time_stamp <= ts && ts <= max_time_stamp)
    {
        // within civil calendar range
        cout << ts << '\n';
    }
    else
    {
        // not within civil calendar range
        auto y = (floor<seconds>(ts) - floor<seconds>(system_clock::now()))/years{1};
        if (y < 0)
            cout << -y << " years ago\n";
        else
            cout << y << " years from now\n";
    }

Despite my years of experience with chrono, I have yet to come up with a one-size-fits-all in the department of overflow checking.

Update: Even with me it took me a couple of iterations of testing corner cases to get the right output outside of the civil calendar range. Just gotta' keep testing! :-)