# \`findmax\` and friends: confusing behaviour to be introduced in 1.7

**URL:** <https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904>\
**Category:** Internals & Design\
**Created:** [May 27, 2021, 7:23am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904 "2021-05-27T07:23:32Z")\
**Posts on this page:** 20\
**Page:** 3

<div class="post-metadata">

**Author:** ![ColinCaine](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/colincaine/32/7211_2.png) [@ColinCaine](https://discourse.julialang.org/u/ColinCaine)\
**Post date:** [May 27, 2021, 4:29pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/41 "2021-05-27T16:29:50Z")

</div>

> Is there any other place in julia where arrays and functions are treated equivalently?

To my knowledge, no. And I’ll emphasise that they aren’t here either: we don’t define `findmax(f::Function)`.

The PR lets you define `findmax(x)` as returning a tuple of a value in the codomain of x and domain of x, and defines a method for indexable collections because the only two reasonable definitions of the `domain` of an indexable collection are “its keys” and “nothing”'; and if we assume “nothing”, then that’s less useful.

I acknowledge that it’s a little unusual (and we talked about it a bit in the PR), but the rules are simple and well described by the docstrings and the solution we came to in the PR is the only one that I could think of that satisfies all of:

- makes sense
- maintains relationships between `min`, `minimum`, `findmin`, `argmin` for all variants and arguments
- doesn’t introduce new functions

It’s quite possible that someone will come up with something better that fits the same requirements, or a better solution that loosens one of those requirements. I think there’s still time to revert my PR and possibly land a new one, if so.

---

<div class="post-metadata">

**Author:** ![Skoffer](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/skoffer/32/378_2.png) [@Skoffer](https://discourse.julialang.org/u/Skoffer)\
**Post date:** [May 27, 2021, 4:40pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/42 "2021-05-27T16:40:11Z")

</div>

> The PR lets you define `findmax(x)` as returning a tuple of a value in the codomain of x and domain of x, and defines a method for indexable collections because the only two reasonable definitions of the `domain` of an indexable collection are “its keys” and “nothing”’; and if we assume “nothing”, then that’s less useful.

But you cannot define `findmax(::Array)` since it’s type piracy, right? And undoubtedly, findmax over arrays is a very obvious and frequent usage.

---

<div class="post-metadata">

**Author:** ![ColinCaine](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/colincaine/32/7211_2.png) [@ColinCaine](https://discourse.julialang.org/u/ColinCaine)\
**Post date:** [May 27, 2021, 4:40pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/43 "2021-05-27T16:40:11Z")

</div>

> [@Skoffer](#):
>
> I do not think, that anyone is saying that nightly `findmax` is useless or bad some other way.

Sure, I get that. I prefer the names and implementations as they are in 1.7, but I think it is okay if the balance of opinion is that this is too confusing and so something else has to be done.

---

<div class="post-metadata">

**Author:** ![ColinCaine](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/colincaine/32/7211_2.png) [@ColinCaine](https://discourse.julialang.org/u/ColinCaine)\
**Post date:** [May 27, 2021, 4:42pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/44 "2021-05-27T16:42:46Z")

</div>

> [@Skoffer](#):
>
> But you cannot define `findmax(::Array)` since it’s type piracy, right? And undoubtedly, findmax over arrays is a very obvious and frequent usage.

I don’t understand why you would want to? The existing definitions of `findmax` for arrays gives a good definition.

---

<div class="post-metadata">

**Author:** ![DNF](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/dnf/32/10191_2.png) [@DNF](https://discourse.julialang.org/u/DNF)\
**Post date:** [May 27, 2021, 5:36pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/45 "2021-05-27T17:36:07Z")

</div>

I had expected

```julia
findmax(f, xs) 

```

to be a fast version of

```julia
findmax(f.(xs))

```

and since getting the actual index will be difficult, the latter is what will be used, hurting my eyes.

Not too pleased with the planned design.

---

<div class="post-metadata">

**Author:** ![tbeason](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tbeason/32/15898_2.png) [@tbeason](https://discourse.julialang.org/u/tbeason)\
**Post date:** [May 27, 2021, 5:45pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/46 "2021-05-27T17:45:42Z")

</div>

To add: I like the idea of extending base functions to work with general iterables, and I am wishing that many packages would follow suit. However, if one takes the view that `findmax` returns indexes (as it always has), it isn’t clear that it should work for general iterables (which aren’t guaranteed to support `getindex`). This is more support in favor of making the new behavior live in a different function.

---

<div class="post-metadata">

**Author:** ![aplavin](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/aplavin/32/222056_2.png) [@aplavin](https://discourse.julialang.org/u/aplavin)\
**Post date:** [May 27, 2021, 7:49pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/47 "2021-05-27T19:49:04Z")

</div>

> [@ColinCaine](#):
>
> The PR lets you define `findmax(x)` as returning a tuple of a value in the codomain of x and domain of x

This may be a useful function to have, but it seems completely independent in its underlying meaning from the current `findmax(A)`. Currently, the latter is described as:

> Return the maximum element of the collection itr and its index or key.

I strongly believe that the natural generalization - if there is any - should follow lines similar to `findfirst` (and others):

> findfirst(A)  
> Return the index or key of the first true value in A.

> findfirst(predicate::Function, A)  
> Return the index or key of the first element of A for which predicate returns true.

That is, the second method is also interpreted in terms of a collection and finding its element subject to some function.  
It’s a sure bet that many more people familiar with existing julia functions would expect `findmax(f, A)` (if it exists) to also return an index into `A` - as `findmax(A)` does. The domain/codomain interpretation seems kinda niche, more specific to mathematical optimization than to general array operations.

> [@ColinCaine](#):
>
> the solution we came to in the PR is the only one that I could think of that satisfies all of:
> 
> - makes sense
> - maintains relationships between `min` , `minimum` , `findmin` , `argmin` for all variants and arguments
> - doesn’t introduce new functions

`findmax(f, A) == findmax(map(f, A))` ticks all of these bullets as well, doesn’t it?  
Additionally, it is consistent with other julia base functions, as pointed out multiple times in this thread.

---

<div class="post-metadata">

**Author:** ![cdawg](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/cdawg/32/9811_2.png) [@cdawg](https://discourse.julialang.org/u/cdawg)\
**Post date:** [May 27, 2021, 10:26pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/48 "2021-05-27T22:26:14Z")

</div>

@ColinCaine I just want to thank you for the PR despite the frustration. It’s a nontrivial amount of unpaid labor in a hopelessly tangled thicket. You mentioned this desiderata:

> - doesn’t introduce new functions

Here’s my opinion: longterm this whole corner of Julia needs to be deprecated, thrown to the bottom of the ocean and replaced with unambiguous function names. For instance: minind, minelt, firstind, firstelt. Then argmin is fine if we have these others (returns whatever went into the function application so f(argmin(f, itr)) = minelt(f, itr)). Otherwise we’ll be ?-ing them every. single. time. The best thing we can do is introduce new unambiguous names now.

---

<div class="post-metadata">

**Author:** ![tkf](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tkf/32/17635_2.png) [@tkf](https://discourse.julialang.org/u/tkf)\
**Post date:** [May 28, 2021, 2:07am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/49 "2021-05-28T02:07:44Z")

</div>

From reading the reactions, I feel it’d be reasonable to revert the PR. We’ll gain consistency at the cost of generality. On one hand, with cmcaine’s PR, we have `findmax(f, ::Channel)` etc. and it still is _ **a** _ generalization of unary `findmax` because `findmax(xs) == findmax(i -> xs[i], eachindex(xs))` (in the sense that one can implemented in terms of the other \[1\]). We also can access index/key by using `findmax(f ∘ last, pairs(xs))`. On the other hand, I find that this argument:

> [@aplavin](#):
>
> Currently, any (?) reduction of the form `r(f, A)` in base julia is just a more efficient form of `r(map(f, A))` .

is compelling.

See also the new comment in GitHub:

> triage thinks 1 arg `argmax` is correct, but `findmin` with a function is broken and needs to be fixed before 1.7.
> 
> — [Unintuitive findmin and findmax · Issue #39203 · JuliaLang/julia · GitHub](https://github.com/JuliaLang/julia/issues/39203#issuecomment-849898668)

* * *

* * *

1. Further “generalizing” this discussion, maybe the main lesson here (for me) is that this definition of generalization was too weak for constructing a good API.

---

<div class="post-metadata">

**Author:** ![aplavin](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/aplavin/32/222056_2.png) [@aplavin](https://discourse.julialang.org/u/aplavin)\
**Post date:** [May 28, 2021, 6:07am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/50 "2021-05-28T06:07:46Z")

</div>

It’s great to see a more “official” reaction to this!  
I personally have trouble understanding what this means specifically:

> [@tkf](#):
>
> See also the new comment in GitHub:
> 
> > triage thinks 1 arg `argmax` is correct, but `findmin` with a function is broken and needs to be fixed before 1.7.

Like, what does this discussion have to do with 1-arg `argmax` at all? It’s definition is effectively set in stone until julia 2 anyway.  
Anyone can clarify?.. @Oscar_Smith?

I just hope this doesn’t end up with keeping `argmax(f, A)` as it is defined in nightly, and changing `findmax(f, A) == findmax(map(f, A))` at the same time. It would be an improvement, of course, because one of the underlying issues is solved:

> [@aplavin](#):
>
> First: we still have no effecient way to find array index of the maximal value wrt some function!

However, such change would still keep these functions inconsistent: `findmax(A)[2] == argmax(A)` but `findmax(f, A)[2] !== argmax(f, A)`, and `argmax(identity, A) != argmax(A)`.

---

<div class="post-metadata">

**Author:** ![Oscar\_Smith](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/oscar_smith/32/25343_2.png) [@Oscar\_Smith](https://discourse.julialang.org/u/Oscar_Smith)\
**Post date:** [May 28, 2021, 6:27am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/51 "2021-05-28T06:27:28Z")

</div>

As I understand it `argmax(f,A)` as defined in nightly is just the mathematical definition. Is this wrong? The reason 1 arg `argmax` was brought up is that since the 2 arg version is mathematically correct, if there is inconsistency, it would be the 1 arg version that’s inconsistent.

The reason `argmax(identity, A) != argmax(A)` is true is because in cases where you want this behavior from `argmax(A)` you can just use `maximum` instead, and it is conceptually really useful in many cases to interpret an `Array` as a function who’s range is the values, and who’s domain is the indices. One possible solution for Julia 2.0 is to deprecate `argmax(A)` since it has 2 somewhat contradictory interpretations, and both of them can be written relatively simply in ways that avoid the confusion.

---

<div class="post-metadata">

**Author:** ![aplavin](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/aplavin/32/222056_2.png) [@aplavin](https://discourse.julialang.org/u/aplavin)\
**Post date:** [May 28, 2021, 6:51am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/52 "2021-05-28T06:51:36Z")

</div>

> [@Oscar\_Smith](#):
>
> As I understand it `argmax(f,A)` as defined in nightly is just the mathematical definition. Is this wrong? The reason 1 arg `argmax` was brought up is that since the 2 arg version is mathematically correct, if there is inconsistency, it would be the 1 arg version that’s inconsistent.

In a vacuum, this would clearly be true.  
But consistency arguments here in this thread apply to `argmax` as well : it’s a reduction, and reductions in base julia obey `r(identity, A) == r(A)` if `r(f, A)` is defined. I started this topic with `findmax` just because this is the function I use most often.  
I’m not aware of any `argmax` overloads in optimization packages, which practically shows that conflating these two meanings (array ops and mathematical optimization) doesn’t look natural.

There is a function that returns the index of the maximal element in an array. This function is called `argmax` in julia. Is it a case of unfortunate naming? Yes. But this is the only meaning currently assigned to `argmax`.  
If `argmax(f, A)` definition is confusing - and looks like it is, the math and array style definitions conflict - maybe it just shouldn’t be defined at all? MethodError would be much better than confusion like

```julia
A = -10:5
ix = argmax(A)
...
A[ix]

# vs

ix = argmax(abs, A)
...
A[ix] # may even work, but give unexpected results

```

> [@Oscar\_Smith](#):
>
> The reason `argmax(identity, A) != argmax(A)` is true is because in cases where you want this behavior from `argmax(A)` you can just use `maximum` instead

`maximum` doesn’t return the index into array, which is the whole point of `argmax` now.

---

<div class="post-metadata">

**Author:** ![FPGro](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/fpgro/32/20822_2.png) [@FPGro](https://discourse.julialang.org/u/FPGro)\
**Post date:** [May 28, 2021, 8:27am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/53 "2021-05-28T08:27:18Z")

</div>

I second @aplavin 's concerns here. It’s clear that the definition that is proposed now is _mathematically_ correct but it pretty much not in line with everything else that is already in place. As much as I do support the change of definitions, I think that this is material for v2.0. (it breaks not only code (potentially), but also developers assumptions about what these functions do (as evidenced by this thread))

For the mean time: What about introducing a set of functions `generalizes_foo` where `foo` is any of the members of the offensing function families (`arg...`, `find...`, more?), that is consistent with the definition that is introduced by @ColinCaine 's PR? (or maybe have homonymous functions live inside a submodule like `GeneralizedReductions`). This would allow us to keep the PR, introduce more correct versions of those functions while being clear that they are different from the usual computer science puns, people could depend on them and Julia v2.0 would simply need a change of names later. ?

(Sidenote: That’s a great discussion to have and a great PR nontheless, thanks y’all)

---

<div class="post-metadata">

**Author:** ![cdawg](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/cdawg/32/9811_2.png) [@cdawg](https://discourse.julialang.org/u/cdawg)\
**Post date:** [May 28, 2021, 8:31am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/54 "2021-05-28T08:31:02Z")

</div>

The problem is isn’t just consistency. It’s the name. Findmax could mean any of the the things proposed and many more. Therefore we should throw away this toxic name. I know it’s hard to swallow that much code breaking but it’s terrible. Look at all this discourse. We’re like college students arguing about Free Will.

Haven’t you read Ursula K. Le Guinn? Find the true names for the functions we need. Deprecate findmax and wait till 2.0. What is this monstrosity doing in base at 1.7?

---

<div class="post-metadata">

**Author:** ![aplavin](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/aplavin/32/222056_2.png) [@aplavin](https://discourse.julialang.org/u/aplavin)\
**Post date:** [May 28, 2021, 11:40am UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/55 "2021-05-28T11:40:40Z")

</div>

> [@FPGro](#):
>
> For the mean time: What about introducing a set of functions `generalizes_foo` where `foo` is any of the members of the offensing function families ( `arg...` , `find...` , more?), that is consistent with the definition that is introduced by @ColinCaine 's PR? (or maybe have homonymous functions live inside a submodule like `GeneralizedReductions` ). This would allow us to keep the PR, introduce more correct versions of those functions while being clear that they are different from the usual computer science puns, people could depend on them and Julia v2.0 would simply need a change of names later. ?

Sounds like a perfect usecase for a package! It would even allow these functions to have their mathematically-expected names without worrying about conflicts: just call them in a qualified way, `Generalized.argmax`.

---

<div class="post-metadata">

**Author:** ![Tamas\_Papp](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tamas_papp/32/25949_2.png) [@Tamas\_Papp](https://discourse.julialang.org/u/Tamas_Papp)\
**Post date:** [May 28, 2021, 2:20pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/56 "2021-05-28T14:20:28Z")

</div>

> [@aplavin](#):
>
> Sounds like a perfect usecase for a package!

Yes, a lot of these utilities could live perfectly well in a package, except those which are used by `Base` and the standard libraries.

Incidentally, I also don’t get the reason for all those `foo(f, itr)` methods when `foo(Iterators.map(f, itr))` and similar sould do just as well.

---

<div class="post-metadata">

**Author:** ![mcabbott](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/mcabbott/32/6603_2.png) [@mcabbott](https://discourse.julialang.org/u/mcabbott)\
**Post date:** [May 28, 2021, 3:19pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/57 "2021-05-28T15:19:36Z")

</div>

> [@Tamas\_Papp](#):
>
> I also don’t get the reason for all those `foo(f, itr)` methods when `foo(Iterators.map(f, itr))` and similar sould do

Note that you can’t do `sum(Iterators.map(abs2, randn(3,4)), dims=1)`, but you can do `sum(abs2, randn(3,4), dims=1)`. (But this doesn’t apply to `findmax` and friends.)

---

<div class="post-metadata">

**Author:** ![Tamas\_Papp](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tamas_papp/32/25949_2.png) [@Tamas\_Papp](https://discourse.julialang.org/u/Tamas_Papp)\
**Post date:** [May 28, 2021, 3:29pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/58 "2021-05-28T15:29:53Z")

</div>

I think you can do this with eg

> **[GitHub - JuliaArrays/LazyArrays.jl: Lazy arrays and linear algebra in Julia](https://github.com/JuliaArrays/LazyArrays.jl)**
>
> Lazy arrays and linear algebra in Julia. Contribute to JuliaArrays/LazyArrays.jl development by creating an account on GitHub.

and

> **[GitHub - bramtayl/JuliennedArrays.jl: Type stable array slicing](https://github.com/bramtayl/JuliennedArrays.jl/)**
>
> Type stable array slicing. Contribute to bramtayl/JuliennedArrays.jl development by creating an account on GitHub.

I am not sure there is any real reason for named variants of `reduce` to take function and slicing arguments at this point, they may just be historical remnants.

---

<div class="post-metadata">

**Author:** ![DNF](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/dnf/32/10191_2.png) [@DNF](https://discourse.julialang.org/u/DNF)\
**Post date:** [May 28, 2021, 4:03pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/59 "2021-05-28T16:03:57Z")

</div>

> [@Tamas\_Papp](#):
>
> Incidentally, I also don’t get the reason for all those `foo(f, itr)` methods when `foo(Iterators.map(f, itr))` and similar sould do just as well.

That’s pretty verbose, and not very discoverable. I don’t think many people would discover or use this.

Having to know about packages like LazyArrays or JuliennedArrays raises the bar even further. Having a terse and trivially discoverable way of doing this is important, imho.

---

<div class="post-metadata">

**Author:** ![Satvik](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/satvik/32/20486_2.png) [@Satvik](https://discourse.julialang.org/u/Satvik)\
**Post date:** [May 28, 2021, 4:38pm UTC](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904/60 "2021-05-28T16:38:06Z")

</div>

`foo(f, itr)` also allows you to use the `do` syntax sugar.

[Previous page](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904.md?page=2)

[Next page](https://discourse.julialang.org/t/findmax-and-friends-confusing-behaviour-to-be-introduced-in-1-7/61904.md?page=4)