D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 3561 - math.abs signature accepts static arrays, but errors internally.
Summary: math.abs signature accepts static arrays, but errors internally.
Status: RESOLVED WONTFIX
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P2 enhancement
Assignee: No Owner
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2009-11-30 13:10 UTC by Rob Jacques
Modified: 2015-11-03 20:43 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Rob Jacques 2009-11-30 13:10:14 UTC
I ran into this while converting a Vector struct to the new value type static arrays. Essentially, float[3] x; x.abs; Errors inside of the abs function. The simple solution is to add !isArray!Num to the list of template constraints. However, since I feel it's useful, I've also included a patch to add support for static arrays to the abs function.

/***********************************
 * Calculates the absolute value
 *
 * For complex numbers, abs(z) = sqrt( $(POWER z.re, 2) + $(POWER z.im, 2) )
 * = hypot(z.re, z.im).
 */
pure nothrow Num abs(Num)(Num x)
    if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) &&
        !isDynamicArray!Num &&
            !(is(Num* : const(ifloat*)) || is(Num* : const(idouble*))
                    || is(Num* : const(ireal*))))
{
    static if (isFloatingPoint!(Num))
        return fabs(x);
    else static if(isStaticArray!Num) {
        typeof(return) a;
        foreach(i,ref b; a)
            b = abs(x[i]);
        return a;
    } else
        return x>=0 ? x : -x;
}

pure nothrow auto abs(Num)(Num z)
    if (is(Num* : const(cfloat*)) || is(Num* : const(cdouble*))
            || is(Num* : const(creal*)))
{
    return hypot(z.re, z.im);
}

/** ditto */
pure nothrow real abs(Num)(Num y)
    if (is(Num* : const(ifloat*)) || is(Num* : const(idouble*))
            || is(Num* : const(ireal*)))
{
    return fabs(y.im);
}


unittest
{
    assert(isIdentical(abs(-0.0L), 0.0L));
    assert(isNaN(abs(real.nan)));
    assert(abs(-real.infinity) == real.infinity);
    assert(abs(-3.2Li) == 3.2L);
    assert(abs(71.6Li) == 71.6L);
    assert(abs(-56) == 56);
    assert(abs(2321312L)  == 2321312L);
    assert(abs(-1+1i) == sqrt(2.0));
    assert(abs([-1,-2,-3]) == [1,2,3]);
}
Comment 1 Andrej Mitrovic 2012-12-21 10:15:58 UTC
It doesn't error internally anymore. But I'm keeping it open for the enhancement request for static array support. Btw your patch doesn't seem to work for this test-case anymore:

assert(abs([-1,-2,-3]) == [1,2,3]);
Comment 2 Andrei Alexandrescu 2015-11-03 19:29:36 UTC
Any chance someone could take this work into a PR? Thanks!
Comment 3 Vladimir Panteleev 2015-11-03 19:58:32 UTC
Why the heck does abs even need to support static arrays or dynamic arrays at all? Should other functions in std.math support arrays too, by that logic? E.g. if abs, why not sin, log, sgn etc.? Just use std.algorithm.map + either std.array.array or std.algorithm.copy, or heck even std.algorithm.each, to apply any unary function over any array.
Comment 4 hsteoh 2015-11-03 20:16:21 UTC
I think this is a bit misplaced. If some people feel the need for std.math functions to work with static arrays, or vectors, or matrices, or whatever else their application needs, couldn't they just implement their own overloads for this purpose?  I'm not sure I see the need for abs(), or any other math function, to have built-in support for static arrays, or any other conglomerate types.  I'm sure some applications out there would love to have this, but it seems to be a rather niche need.
Comment 5 Andrei Alexandrescu 2015-11-03 20:27:00 UTC
I'm unclear what to do on enhancement requests that contain small ideas like this. If this were a PR, I'd consider it. But as things are, we need a champion to take this to a PR.
Comment 6 Vladimir Panteleev 2015-11-03 20:34:05 UTC
Let's just close. This was requested 6 years ago. Today a better approach would be a template function which e.g. applies an unary function over a static array. Hard-coding the functionality into abs seems like a suboptimal direction to take today, as there is no special logic to this that is specific to abs and would not apply to any other unary function.
Comment 7 hsteoh 2015-11-03 20:42:31 UTC
Precisely. D has enough machinery to be able to factor out the common logic of applying a unary function over a static array, dynamic array, matrix, etc., there's no need to complicate the implementation of abs with something that isn't even scalable in the long run (have to implement looping over array / vector for every unary function in std.math, with possibility of bugs each time, plus added maintenance costs).
Comment 8 Vladimir Panteleev 2015-11-03 20:43:47 UTC
(In reply to Vladimir Panteleev from comment #6)
> Hard-coding the functionality into abs seems like a suboptimal
> direction to take today, as there is no special logic to this that is
> specific to abs and would not apply to any other unary function.

To expand on this: we don't have an overload of max that takes an array because that's what reduce!max does. There could be a similar apply function where apply!abs does what the OP asked.