chrono::duration_cast problems during ratio multiplication

Question

I am writing some code that has to deal with NTP times. I decided to represent them using std::chrono::duration, which I hoped would make my time math easier to do.

An NTP time is represented by a unsigned 64-bit integer, with the seconds since the epoch in the high 32 bits, and the fractional seconds in the low 32 bits. The epoch date is January 1, 1900, but that is a separate problem from the one I am dealing with here, and I already know how to deal with that. For the examples I am working with, assume an epoch of January 1, 1970 to make the math simpler.

The duration representation was straightforward: std::chrono::duration<std::uint64_t, std::ratio<1, INTMAX_C(0x100000000)>>. The problem lay in when I was trying to convert between NTP times and my system clock.

On my system, the std::chrono::system_clock::duration is std::chrono::duration<int64_t, std::nano>. When I tried to use std::chrono::duration_cast to cast between these two durations, I got massive truncation in either direction. Tracing this in the debugger, I found it had to do with the duration_cast implementation, which fails in the same way in libstdc++, libc++, and boost::chrono. In short, to convert from system to ntp representation, it multiples by the ratio 8388608/1953125, and the reverse ratio in the other direction. It first multiplies by the numerator, then divides by the denominator. The math is correct, but the initial multiplication overflows the 64-bit representation and gets truncated, despite the fact that actual conversion (post-division) is still easily representable in those bits.

I can do this conversion manually, but my questions are as follows:

1. Is this a bug in the implementations, or just a limitation?
2. If a limitation, should I have realized this would be a problem? I didn't see any documentation that would seem to lead to being able to guess that this would not work.
3. Is there a generic way to implement this that isn't prey to the same problem?
4. Should this be reported, and to whom?
5. Finally, if, when C++20 comes around, I use an NTP clock with manually curated to_sys and from_sys functions, will I simply be able to use std::chrono::clock_cast and not have to worry about other subtle problems?

Here is the code I used to test this and some sample output:

// #define BOOST
#ifdef BOOST
#  include <boost/chrono.hpp>
#  define PREFIX boost
#else
#  include <chrono>
#  define PREFIX std
#endif
#include <cstdint>
#include <iostream>

namespace chrono = PREFIX::chrono;
using PREFIX::ratio;
using PREFIX::nano;

using ntp_dur = chrono::duration<uint64_t, ratio<1, INTMAX_C(0x100000000)>>;
using std_dur = chrono::duration<int64_t, nano>;

int main() {
  auto write = [](auto & label, auto & dur) {
    std::cout << label << ": "
              << std::dec << dur.count()
              << std::hex << " (" << dur.count() << ")" << std::endl;
  };

  auto now = chrono::system_clock::now().time_since_epoch();
  write("now", now);
  std::cout << '\n';

  std::cout << "Naive conversion to ntp and back\n";
  auto a = chrono::duration_cast<std_dur>(now);
  write("std", a);
  auto b = chrono::duration_cast<ntp_dur>(a);
  write("std -> ntp", b);
  auto c = chrono::duration_cast<std_dur>(b);
  write("ntp -> std", c);
  std::cout << '\n';

  std::cout << "Broken down conversion to ntp, naive back\n";
  write("std", a);
  auto d = chrono::duration_cast<chrono::seconds>(a);
  write("std -> sec", d);
  auto e = chrono::duration_cast<ntp_dur>(d);
  write("sec -> ntp sec", e);
  auto f = a - d;
  write("std -> std frac", f);
  auto g = chrono::duration_cast<ntp_dur>(f);
  write("std frac -> ntp frac", f);
  auto h = e + g;
  write("ntp sec + ntp frac-> ntp", h);
  auto i = chrono::duration_cast<std_dur>(h);
  write("ntp -> std", i);
  std::cout << '\n';

  std::cout << "Broken down conversion to std from ntp\n";
  write("ntp", h);
  auto j = chrono::duration_cast<chrono::seconds>(h);
  write("ntp -> sec", j);
  auto k = chrono::duration_cast<std_dur>(j);
  write("src -> std sec", j);
  auto l = h - j;
  write("ntp -> ntp frac", l);
  auto m = chrono::duration_cast<std_dur>(l);
  write("ntp frac -> std frac", m);
  auto n = k + m;
  write("std sec + std frac-> std", n);
}

Sample output:

now: 1530980834103467738 (153f22f506ab1eda)

Naive conversion to ntp and back
std: 1530980834103467738 (153f22f506ab1eda)
std -> ntp: 4519932809765 (41c60fd5225)
ntp -> std: 1052378865369 (f506ab1ed9)

Broken down conversion to ntp, naive back
std: 1530980834103467738 (153f22f506ab1eda)
std -> sec: 1530980834 (5b40e9e2)
sec -> ntp sec: 6575512612832804864 (5b40e9e200000000)
std -> std frac: 103467738 (62acada)
std frac -> ntp frac: 103467738 (62acada)
ntp sec + ntp frac-> ntp: 6575512613277195414 (5b40e9e21a7cdc96)
ntp -> std: 1052378865369 (f506ab1ed9)

Broken down conversion to std from ntp
ntp: 6575512613277195414 (5b40e9e21a7cdc96)
ntp -> sec: 1530980834 (5b40e9e2)
src -> std sec: 1530980834 (5b40e9e2)
ntp -> ntp frac: 444390550 (1a7cdc96)
ntp frac -> std frac: 103467737 (62acad9)
std sec + std frac-> std: 1530980834103467737 (153f22f506ab1ed9)

Answer 1

1. Is this a bug in the implementations, or just a limitation?

Just a limitation. The implementations are behaving according to spec.

2. If a limitation, should I have realized this would be a problem? I didn't see any documentation that would seem to lead to being able to guess that this would not work.

The specification is in 23.17.5.7 [time.duration.cast] of the C++ standard. It documents the conversion algorithm to behave as you describe in your question.

3. Is there a generic way to implement this that isn't prey to the same problem?

When dealing with either very fine units, or very large ranges, you need to be aware of the potential for overflow error. chrono::duration_cast is designed to be the lowest-level tool to handle the most common conversions as efficiently as possible. chrono::duration_cast is accurate as often as it can be, completely eliminating divisions whenever possible.

However no conversion algorithm can always get the right answer to arbitrary conversions using a limited amount of storage. C++17 introduces three new conversion algorithms that build on duration_cast and are meant to guide the direction of truncation when it exists:

floor  // truncate towards negative infinity
ceil   // truncate towards positive infinity
round  // truncate towards nearest, to even on tie

You can write your own generic conversion functions to handle difficult situations such as the one you describe. Such a client-supplied conversion is not likely to be appropriate for general purpose use. For example:

template <class DOut, class Rep, class Period>
DOut
via_double(std::chrono::duration<Rep, Period> d)
{
    using namespace std::chrono;
    using dsec = duration<long double>;
    return duration_cast<DOut>(dsec{d});
}

The above example client-supplied conversion bounces off of duration<long double>. This is less prone to overflow (though not immune), will generally be more expensive computationally, and will suffer from precision problems (and depends on numeric_limits<long double>::digits).

For me (numeric_limits<long double>::digits == 64), an input of 1530996658751420125ns round trips to 1530996658751420124ns (off by 1ns). This algorithm can be improved by the use of round over duration_cast (again at the cost of more computation):

template <class DOut, class Rep, class Period>
DOut
via_double(std::chrono::duration<Rep, Period> d)
{
    using namespace std::chrono;
    using dsec = duration<long double>;
    return round<DOut>(dsec{d});
}

Now my round-trip is perfect for an input of 1530996658751420125ns. However if your long double only has 53 bits of precision, not even round will provide a perfect round-trip.

4. Should this be reported, and to whom?

Anyone can file a defect report against the library half of the C++ standard by following the instructions at this link. Such a report will be looked at by the LWG subcommittee of the C++ standards committee. It may be acted upon, or may be declared NAD (not a defect). If an issue contains proposed wording (detailed instructions on how you would like the specification to be changed), it will have a higher chance of success.

I.e. What do you think the standard should say?

5. Finally, if, when C++20 comes around, I use an NTP clock with manually curated to_sys and from_sys functions, will I simply be able to use std::chrono::clock_cast and not have to worry about other subtle problems?

One way to find out is to experiment with this existing prototype of the C++20 draft specification for the <chrono> extensions.