std.algorithm.search: expose extremum

[wilzbach]

On #4221 the consensus was that extremum is rarely needed, hence we set it to private.

It seems like @9il just found a nice use case for extremum, std.math.cmp:

assert([-2.0, 2, 4].reduce!`cmp(a, b) ? a : b` == -2.0);
assert([-2.0, 2, 4].reduce!`cmp(a, b) ? b : a` == 4.0);

Well it's also possible to use reduce:

assert([-1., 0, 1].extremum!cmp == 1.0);
assert([-1., 0, 1].extremum!`cmp(a, b) < 0` == -1.0);

However it gets neat, if map is needed:

assert([-1., 0, 1].enumerate.extremum!cmp == tuple(2, 1.0));
assert([-1., 0, 1].enumerate.extremum!`cmp(a, b) < 0` == tuple(0, -1.0));

I added a small goodie that checks whether the first template function can be used as an mapping function (unary function) and otherwise it falls back to the selector (binary function).

Additionally I added more tests.

[wilzbach]

Btw during the minElement review no one complained, but is there a way to avoid the mapping element if a "a" noop function is used?

[9il]

ping @DmitryOlshansky

[wilzbach]

Btw during the minElement review no one complained, but is there a way to avoid the mapping element if a "a" noop function is used?

Yeah got it too work, so that now a IdentityFun is passed as default argument from extremum and {min,max}Element and statically checked against in the extremum function :)

[wilzbach]

Yeah got it too work, so that now a IdentityFun is passed as default argument from extremum and {min,max}Element and statically checked against in the extremum function

Should I separate this optimization? (it doesn't depend on extremum being exposed to the public)

[9il]

What optimization and why do you think that this is optimization?

[wilzbach]

What optimization and why do you think that this is optimization?

If the alias is "a" (e.g. [0,2,3].minElement), currently this is executed:

MapType mapElement = mapFun(r[i]);
if (selectorFun(mapElement, extremeElementMapped))
{
   extremeElement = r[i];
   extremeElementMapped = mapElement;
}

this PR reduces it down to:

if (selectorFun(r[i], extremeElement))
{
   extremeElement = r[i];
}

(have a look at the changes)

[wilzbach]

Should I separate this optimization? (it doesn't depend on extremum being exposed to the public)

See #4265 ;-)

[9il]

If the alias is "a" (e.g. [0,2,3].minElement), currently this is executed:

MapType mapElement = mapFun(r[i]);
if (selectorFun(mapElement, extremeElementMapped))
{
extremeElement = r[i];
extremeElementMapped = mapElement;
}
this PR reduces it down to:

if (selectorFun(r[i], extremeElement))
{
extremeElement = r[i];
}
(have a look at the changes)

they are equivalent, are not they? (i mean assembler code)

[wilzbach]

they are equivalent, are not they? (i mean assembler code)

Have a look: http://www.mergely.com/966byvI1/ - (e.g. unaryFun is called on the right)

[9il]

they are equivalent, are not they? (i mean assembler code)
Have a look: http://www.mergely.com/966byvI1/ - (e.g. unaryFun is called on the right)
Hide all checks

Try with ldc2 -O -release -boundscheck=off -output-s

[andralex]

Can't we define overloads to avoid map if not specified? Should we promote only to package level?

[wilzbach]

Can't we define overloads to avoid map if not specified?

See #4265.

Should we promote only to package level?

The code has been moved to #4265 - this is just about whether extremum should be publicly exposed.
See my first post for an example for which extremum could be useful.

[andralex]

/// before this?

[wilzbach]

this is just about whether extremum should be publicly exposed.
Please see my first post for an example for which extremum could be useful.

Rebased to latest master. So do we want to expose extremum?

[andralex]

I approve the addition, seeing I have a soft spot for extremum too. @wilzbach could you please detect the identity mapping via a specialization (as opposed to string comparison)? Seems more elegant. LMK.

[wilzbach]

andralex commented on Dec 24, 2016
I approve the addition, seeing I have a soft spot for extremum too.

While it would have been a nice Christmas present, after thinking about this for a while I reached my internal consensus that the use cases of a public extremum would be pretty low and exposing it to the general public would lead to more confusion than help.
Using reduce works just fine and if real use cases where it would help to simplify code still arise, we can always revert this decision, but once exposed there's no going back.

@wilzbach could you please detect the identity mapping via a specialization (as opposed to string comparison)? Seems more elegant. LMK.

With #5001 we finally got this in :)