Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please add maxElement and minElement in mir (trying to use std.algorithm gives opCmp not defined) #471

Open
prbuen opened this issue Nov 13, 2024 · 5 comments

Comments

@prbuen
Copy link

prbuen commented Nov 13, 2024

To find the maximum value in an ndslice u, we can use mir's maxIndex:

u[u.maxIndex]

but it would be nice to have the functions maxElement and minElement directly implemented in mir. It's not possible to use these functions from std.algorithm (the complaint is that opCmd is not defined), unless doing u.flattened.maxElement.

It doesn't sound like much but it would allow to more easily write code that can apply to both standard D dynamic arrays and ndslices.

Here is my use case: in numerical computation (e.g. errors) we often need to find the maximum absolute value of an ndslice. This can be achieved both with dynamic arrays (were it not for this bug) and ndslices with

u[u.map!fabs.maxIndex].fabs

but it's pretty heavy and not as readable compared to

u.map!fabs.maxElement

Thanks!

@jmh530
Copy link
Contributor

jmh530 commented Nov 14, 2024

Something like:

/++
Returns the minimal(maximal) element of a multidimensional slice.

Params:
    pred = A predicate.

See_also:
    $(LREF minIndex),
    $(LREF maxElement),
    $(LREF maxIndex),
    $(LREF maxPos).
+/
template minElement(alias pred = "a < b")
{
    import mir.functional: naryFun;
    static if (__traits(isSame, naryFun!pred, pred))
    /++
    Params:
        slice = ndslice.
    Returns:
        Minimal(maximal) element of a multidimensional slice
    +/
    @fmamath size_t[N] minElement(Iterator, size_t N, SliceKind kind)(Slice!(Iterator, N, kind) slice)
    {
        return slice[slice.minIndex!pred];
    }
    else
        alias minElement = .minElement!(naryFun!pred);
}

/// ditto
template maxElement(alias pred = "a < b")
{
    import mir.functional: naryFun, reverseArgs;
    alias maxElement = minElement!(reverseArgs!(naryFun!pred));
}

@prbuen
Copy link
Author

prbuen commented Nov 14, 2024

Thanks, that's great! Compilation fails with cannot implicitly convert expression 'slice.opIndex(minIndex(slice))' of type 'double' to 'ulong[<n>]' on the line

      return slice[slice.minIndex!pred];

but changing the return type of minElement from size_t[N] to auto makes it work.

Both u.maxElement and u.map!fabs.maxElement (and the minElement counterparts) work as expected. Would be great to merge :)
Thanks again!

@jmh530
Copy link
Contributor

jmh530 commented Nov 14, 2024

Yeah, I just kind of threw that together without testing. I'd prefer an actual return type instead of auto.
I don't have a lot of free time these days, but I can try to put together the PR soon-ish.

@prbuen
Copy link
Author

prbuen commented Nov 14, 2024

That would be great. I agree with preferring an actual type but don't feel confident enough here to know how to write it. FWIW, std.algorithm.maxElement does return auto.

@jmh530
Copy link
Contributor

jmh530 commented Nov 28, 2024

@prbuen Implementation here: #473

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants