std::abs() on std::chrono::duration

[Nadav Har'El]

Hi, I noticed that it seems the C++11 standard doesn't require std::abs() to be defined for std::chrono::duration.

If I understand correctly, std::abs() is defined on integers (declared in header file <cstdlib>), floats (<cmath>), complex (<complex>), and valarray (<valarray>), but not on std::chrono::duration. Is this deliberate, or an oversight? Any chance this can be added in C++14?

I think the implementation can be as simple as the following - applying std::abs to count():

namespace std {
template<typename Rep,typename Period>
std::chrono::duration<Rep,Period> abs(std::chrono::duration<Rep, Period> d)
{
    return std::chrono::duration<Rep,Period>(std::abs(d.count()));
}
}

[Vicente J. Botet Escriba]

Le 25/01/14 21:49, Nadav Har'El a écrit :

Hi,

yes this will be trivial. But why not for time_points or other wrapping types, as unique_ptr, shared_ptr, optional (expected)?
If these classes overloads a underlying function (see below) we could have

template<class M>
M abs(M const& m) {

    return M(std::abs(underlying(m));
}

and be able to do

d = std::abs(d);
tp = std::abs(tp);

It is worth making more generic this function?

Best,
Vicente

==========
template<class Rep, class Period>
Rep underlying(std::chrono::duration<Rep, Period>const& d)

{
    return d.count();
}

template<class Clock,class Duration>
Duration underlying(std::chrono::time_point<Clock, Duration> const& tp)

{
    return d.time_since_epoch();
}

[Howard Hinnant]

I also think abs(duration) is a good idea. And so is floor, round and ceil. Implementations shown here:

http://home.roadrunner.com/~hinnant/duration_io/chrono_util.html

I don't think it would be a good idea to write abs(duration) in terms of abs(duration::rep) for the following reasons:

1. It would require <chrono> to be dependent on both <cstdlib> and <cmath>. The latter is not that light weight, and

2. There's a fair chance it won't work for non-arithemetic rep.

3. It becomes far more difficult to mark it constexpr when you have to depend on functions from the C lib.

For integral types, the C abs(int) function is no more efficient than relatively naive code. Modern optimizers routinely recognize naive abs code and transform it to the most efficient for the platform. For example:

int
my_abs(int x)
{
return x >= 0 ? x : -x;
}

__Z6my_absi: ## @_Z6my_absi
.cfi_startproc
## BB#0: ## %entry
pushq %rbp
Ltmp2:
.cfi_def_cfa_offset 16
Ltmp3:
.cfi_offset %rbp, -16
movq %rsp, %rbp
Ltmp4:
.cfi_def_cfa_register %rbp
movl %edi, %eax
negl %eax
cmovll %edi, %eax
popq %rbp
retq
.cfi_endproc

I.e. the branch has been eliminated. And it is small enough to be inlined. Because of this latter fact, it is actually faster to call an inlined my_abs(int) than it is to call into a C library.

I also think that abs(unsigned duration) should fail to compile. I believe it to be error prone for the reasons described here:

http://stackoverflow.com/a/21322659/576911

These links also show how to implement the compile-time failure for abs(unsigned duration).

Howard

[Nadav Har'El]

On Mon, Jan 27, 2014 at 2:23 AM, Howard Hinnant <howard....@gmail.com> wrote:

On Jan 26, 2014, at 4:27 PM, "Vicente J. Botet Escriba" <vicent...@wanadoo.fr> wrote:

> Le 25/01/14 21:49, Nadav Har'El a écrit :
>> Hi, I noticed that it seems the C++11 standard doesn't require std::abs() to be defined for std::chrono::duration.

>> I think the implementation can be as simple as the following - applying std::abs to count():
>>
>> namespace std {
>> template<typename Rep,typename Period>
>> std::chrono::duration<Rep,Period> abs(std::chrono::duration<Rep, Period> d)
>> {
>>     return std::chrono::duration<Rep,Period>(std::abs(d.count()));
>> }
>> }
>>
> Hi,
>
> yes this will be trivial. But why not for time_points or other wrapping types, as unique_ptr, shared_ptr, optional (expected)?

Since unique_ptr and shared_ptr emulate a pointer (they need to be dereferenced to get the actual value), implementing abs() on them would be really unexpected.

 

>
> It is worth making more generic this function?
>
> Best,
> Vicente
>
> ==========
> template<class Rep, class Period>
> Rep underlying(std::chrono::duration<Rep, Period>const& d)
> {
>     return d.count();
> }
>
> template<class Clock,class Duration>
> Duration underlying(std::chrono::time_point<Clock, Duration> const& tp)
> {
>     return d.time_since_epoch();
> }

Nice, although having a negative time_point is really strange, I guess for some use-cases it might apply.
 

I also think abs(duration) is a good idea.  And so is floor, round and ceil.  Implementations shown here:

http://home.roadrunner.com/~hinnant/duration_io/chrono_util.html

Nice.
 

I don't think it would be a good idea to write abs(duration) in terms of abs(duration::rep) for the following reasons:

The reason why I wrote duration's abs() in terms of Rep's abs() is that it allows to transparently inherit Rep's abs() implementation, without needing to implement it again for duration. Sure, for any "sane" rep, implementing abs() is trivial. But I wonder if we would ever want an "insane" rep - e.g., for a std::complex Rep, your abs() implementation would be incorrect ;-)

But again, for any sane (integer-like or float-like) Rep type, your implementation is perfectly correct.

 

1.  It would require <chrono> to be dependent on both <cstdlib> and <cmath>.  The latter is not that light weight, and

I don't think it makes <chrono> depend on <cstdlib> or <cmath> - rather, just if you want to use std::abs, you need to include those.

 

2.  There's a fair chance it won't work for non-arithemetic rep.

It won't work if std::abs() isn't implemented on it, but then you probably shouldn't expect it to work!

 

3.  It becomes far more difficult to mark it constexpr when you have to depend on functions from the C lib.

I didn't know that std::abs() is a C library. Why is it? ::abs() should obviously be from the C library, but I didn't know std::abs() was too.

 

I also think that abs(unsigned duration) should fail to compile.  I believe it to be error prone for the reasons described here:

http://stackoverflow.com/a/21322659/576911

Normal abs(unsigned int) is also error-prone for the same reasons, so perhaps for the same reason, std::abs(unsigned int) should be prevented by the same sort of trick? That would make my duration-abs-using-rep-abs implementation inherit such a restriction automatically: because abs(duration.count()) doesn't compile, neither will abs(duration).

I find it surprising that std::abs() will protect you against mistakes with unsigned durations, but not against unsigned integers.

[Daniel Krügler]

2014/1/27 Howard Hinnant <howard....@gmail.com>:

> On Jan 26, 2014, at 4:27 PM, "Vicente J. Botet Escriba" <vicent...@wanadoo.fr> wrote:

[..]

> I also think that abs(unsigned duration) should fail to compile. I believe it to be error prone for the reasons described here:
>
> http://stackoverflow.com/a/21322659/576911

The directions given to

http://cplusplus.github.io/LWG/lwg-active.html#2192

seem to indicate that the committee is intending to add unsigned
overloads for std::abs ;-)

- Daniel

[Howard Hinnant]

On Jan 27, 2014, at 8:46 AM, Daniel Krügler <daniel....@gmail.com> wrote:

> The directions given to
>
> http://cplusplus.github.io/LWG/lwg-active.html#2192
>
> seem to indicate that the committee is intending to add unsigned
> overloads for std::abs ;-)

That would seem most error prone. Here is a non-contrived custom chrono clock I have in my tool box:

namespace x
{

struct clock
{
typedef unsigned long long rep;
typedef std::ratio<1, 2800000000> period; // My machine is 2.8 GHz
typedef std::chrono::duration<rep, period> duration;
typedef std::chrono::time_point<clock> time_point;
static const bool is_steady = true;

static time_point now() noexcept
{
unsigned lo, hi;
asm volatile("rdtsc" : "=a" (lo), "=d" (hi));
return time_point(duration(static_cast<rep>(hi) << 32 | lo));
}

private:

static
unsigned
get_clock_speed()
{
int mib[] = {CTL_HW, HW_CPU_FREQ};
const std::size_t namelen = sizeof(mib)/sizeof(mib[0]);
unsigned freq;
size_t freq_len = sizeof(freq);
if (sysctl(mib, namelen, &freq, &freq_len, nullptr, 0) != 0)
return 0;
return freq;
}

static
bool
check_invariants()
{
static_assert(1 == period::num, "period must be 1/freq");
assert(get_clock_speed() == period::den);
static_assert(std::is_same<rep, duration::rep>::value,
"rep and duration::rep must be the same type");
static_assert(std::is_same<period, duration::period>::value,
"period and duration::period must be the same type");
static_assert(std::is_same<duration, time_point::duration>::value,
"duration and time_point::duration must be the same type");
return true;
}

static const bool invariants;
};

const bool clock::invariants = clock::check_invariants();

} // x

It isn't portable. But when I need a bare-to-the-metal timer, this is what I reach for.

Now consider a world where std::abs(0ull) compiles. I'll simulate such a world with the exp namespace:

namespace exp
{

template <class Int>
Int
abs(Int x)

{
return x >= 0 ? x : -x;
}

}

And finally consider some analysis code that somewhere deep down in the details, is going to manipulate time_points from my x::clock, and call abs on x::clock::rep (really low level, hidden details here):

template <class Clock, class Duration>
typename Duration::rep
analyze(std::chrono::time_point<Clock, Duration> x,
std::chrono::time_point<Clock, Duration> y)
{
return exp::abs((x-y).count());
}

int
main()
{
auto x = x::clock::now();
auto y = x::clock::now();
std::cout << analyze(x, y) << '\n';
}

This all looks quite reasonable, right?

It is a run-time error. For me it prints out:

18446744073709551559

It should be printing out a relatively small number of clock ticks. If I change x::clock::rep from unsigned long long to long long, I get:

18

(the expected result).

But a signed rep for my clock really doesn't make sense. Really the bug is a combination of things:

1. I'm using a clock with an analysis that was not designed to handle unsigned reps.

2. The analysis should have known that it was not designed to handle unsigned reps, detected it, and given a compile-time diagnostic.

One thing the C++ committee could do to help the coder of analyze(), and the client who is using x::clock(), is to not proceed with LWG 2192. I.e. this result is actually a good result:

test.cpp:80:12: error: call to 'abs' is ambiguous
return std::abs((x-y).count());
^~~~~~~~
test.cpp:90:18: note: in instantiation of function template specialization 'analyze<x::clock, std::__1::chrono::duration<unsigned long long, std::__1::ratio<1,
2800000000> > >' requested here
std::cout << analyze(x, y) << '\n';
^
/usr/include/stdlib.h:129:6: note: candidate function
int abs(int) __pure2;
^
../libcxx/include/cstdlib:159:44: note: candidate function
inline _LIBCPP_INLINE_VISIBILITY long abs( long __x) _NOEXCEPT {return labs(__x);}
^
../libcxx/include/cstdlib:161:44: note: candidate function
inline _LIBCPP_INLINE_VISIBILITY long long abs(long long __x) _NOEXCEPT {return llabs(__x);}
^
1 error generated.

Now I'm alerted to the bug at compile time, and I can decide what to do about it.

Howard

[Miro Knejp]

> Now consider a world where std::abs(0ull) compiles. I'll simulate such a world with the exp namespace:
>
> namespace exp
> {
>
> template <class Int>
> Int
> abs(Int x)
> {
> return x >= 0 ? x : -x;
> }
>
> }

Why would anyone in their right mind implement it like that? Especially
people providing the standard library?
In such a world I wouldn't expect any serious std author to do the above
but make std::abs(0ull) a no-op with an overload instead. Of course your
example fails with that nonsense.

Once there are proper overloads for unsigned integral types then it's
best to delegate abs(duration<Rep>) to abs (Rep) as only the latter
knows what's the appropriate way to get the magnitude for Rep.

[Howard Hinnant]

Please take my example (or similar), add the overloads you propose, and demonstrate your superior results.

Howard

[Miro Knejp]

It doesn't matter because the real problem is the analyse method, namely
the expression x - y, not abs, by making the assumption that the
involved types are signed. So the analyse method is already broken if
given unsigned types and x < y. The presence or absence of abs doesn't
change a thing and what if analyse did something completely different
and called sqrt(x - y) or exp(x - y)? No compile error regardless of
signedness and it may still break at runtime. This example is just
cherry picking a corner case for one type in one situation that neglects
all other use cases of abs. If you call x - y in a template method,
better make sure your types are either signed or ordered. Don't use bad
examples of QoI as strawman for unrelated methods.

When there is a function abs(X<T>) I expect it to delegate to abs(T) and
not do any voodoo. And why is this suddenly a discussion about unsigned
abs...

[Howard Hinnant]

On Jan 27, 2014, at 1:21 PM, Miro Knejp <mi...@knejp.de> wrote:

> It doesn't matter because the real problem is the analyse method, namely the expression x - y, not abs, by making the assumption that the involved types are signed. So the analyse method is already broken if given unsigned types and x < y.

That was my point.

> The presence or absence of abs doesn't change a thing

The providing of abs(unsigned) aids in hiding the bug in analyze.

C and C++ have gone for decades without an abs(unsigned). Introducing it now will cause more problems than it solves.

> And why is this suddenly a discussion about unsigned abs...

Because:

On Jan 27, 2014, at 8:46 AM, Daniel Krügler <daniel....@gmail.com> wrote:

> The directions given to
>
> http://cplusplus.github.io/LWG/lwg-active.html#2192
>
> seem to indicate that the committee is intending to add unsigned
> overloads for std::abs ;-)

Howard

[Miro Knejp]

Am 27.01.2014 19:31, schrieb Howard Hinnant:
> The providing of abs(unsigned) aids in hiding the bug in analyze.

So does exp, pow, sqrt and actualy *every* function that takes an
argument implicitly constructible from integers. I don't see the point
in using x - y in combination with chrono as argument against unsigned
abs. That's just special pleading.

>
> C and C++ have gone for decades without an abs(unsigned). Introducing it now will cause more problems than it solves.

And without auto, too. There are enough people claiming it is a source
of bugs. "that's how it's always been" is not an argument. Introduing it
now is unlikely to break existing code because code using unsigned
arguments to abs currently doesn't compile unless user provided
overloads already exist.Still not relted to whether abs(duration) is
goor or not.

[Vicente J. Botet Escriba]

Le 27/01/14 01:23, Howard Hinnant a écrit :

> On Jan 26, 2014, at 4:27 PM, "Vicente J. Botet Escriba"<vicent...@wanadoo.fr> wrote:
>
>> Le 25/01/14 21:49, Nadav Har'El a écrit :
>>> Hi, I noticed that it seems the C++11 standard doesn't require std::abs() to be defined for std::chrono::duration.
>>>
>>> If I understand correctly, std::abs() is defined on integers (declared in header file <cstdlib>), floats (<cmath>), complex (<complex>), and valarray (<valarray>), but not on std::chrono::duration. Is this deliberate, or an oversight? Any chance this can be added in C++14?
>>>
>>> I think the implementation can be as simple as the following - applying std::abs to count():
>>>
>>> namespace std {
>>> template<typename Rep,typename Period>
>>> std::chrono::duration<Rep,Period> abs(std::chrono::duration<Rep, Period> d)
>>> {
>>> return std::chrono::duration<Rep,Period>(std::abs(d.count()));
>>> }
>>> }
>>>
>> Hi,
>>
>> yes this will be trivial. But why not for time_points or other wrapping types, as unique_ptr, shared_ptr, optional (expected)?
>> If these classes overloads a underlying function (see below) we could have
>>
>> template<class M>
>> M abs(M const& m) {
>> return M(std::abs(underlying(m));
>> }
>>
>> and be able to do
>>
>> d = std::abs(d);
>> tp = std::abs(tp);
>>
>> It is worth making more generic this function?
>>
>> Best,
>> Vicente
>>
>>

> I also think abs(duration) is a good idea. And so is floor, round and ceil. Implementations shown here:
>
> http://home.roadrunner.com/~hinnant/duration_io/chrono_util.html

The definition of abs in your implementation is closer to the mathematic
definition and can be generalized to

template<class MP>
MP abs(MP m) {
return m >= zero<MP>() ? m : -m;
}

For the types for which zero exists.

template<class MP, class MP_ = MonadPlus<Cat<MP>>
constexpr MP zero() {
return MP_::template zero<MP>();
}


template<>
struct MoandPlus<builtin_tag>
{
template <class MP>
MP zero() { return 0; }
};

template<typename Rep, typename Period>
struct MoandPlus<std::chrono::duration<Rep,Period>>
{
template <class D>
D zero() { return D::zero(); }
};

Vicente

P.S. See http://yapb-soc.blogspot.fr/2012/10/monads-in-c.html for what
is behind the scenes.