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

Added indmin and indmax #1655

Merged
merged 2 commits into from
Dec 6, 2012
Merged

Added indmin and indmax #1655

merged 2 commits into from
Dec 6, 2012

Conversation

SamChill
Copy link
Contributor

@SamChill SamChill commented Dec 2, 2012

Added argmin and argmax to base/array.jl. Also documented argmin, argmax, findmin, and findmax as well as added tests for argmin and argmax to test/arrayops.jl.

argmax, findmin, and findmax as well as added tests for argmin and
argmax to test/arrayops.jl.
@johnmyleswhite
Copy link
Member

I like both the idea of adding an argmax function and this patch, but I'm not sure I'd call this operation argmax.

Instead it seems like it should be called indmax or something similar.

For me, argmax should involve passing a function: argmax(a::AbstractArray, f::Function) would return the value of a that maximizes f.

@SamChill
Copy link
Contributor Author

SamChill commented Dec 2, 2012

I feel like your definition of argmax doesn't conflict with the one in my patch. Thats the great thing about multiple dispatch. argmax is the name for this function in numpy and matlab.

@johnmyleswhite
Copy link
Member

True, MD does allow both of these to coexist.

And maybe I'm confused, but I think my definition does conflict with yours: I was proposing that you return the value of the array's maximizing element, not the index.

That said, precedent from other languages is a strong argument.

@SamChill
Copy link
Contributor Author

SamChill commented Dec 2, 2012

I might be confused, but the value of the array's maximizing element is what max is.

@johnmyleswhite
Copy link
Member

Yes. And argmax would be the value that maximizes the passed in function, which is not what max does when passed a function right now: max(x -> x^2, [1, 3, 4, 3]) evaluates to 16 instead of 4, which is what I would think argmax ought to do. As I understand your function argmax([2, 3, 4, 3]) evaluates to 3. Or am I missing something obvious?

@SamChill
Copy link
Contributor Author

SamChill commented Dec 2, 2012

I'm a little slow today. I see what you mean now. Since you are passing an array of x values, it makes sense in your definition to return the value of the array passed that maximizes f.

I like both functions: argmax(a::AbstractArray) and argmax(a::AbstractArray, f::Function), but the definition is different between them (returning index versus value). Your's is more like the math definition and mine is the same definition used in numpy and matlab.

I'm not sure what the best way forward is. That can be left to someone more closely tied to the project. indmax is a fine name for the function that I wrote.

@johnmyleswhite
Copy link
Member

Let's see what the core team thinks. I would really like to have ind* functions as well as arg* functions and have actually added them a few times in the past as utility functions for small projects.

@ViralBShah
Copy link
Member

I thought about this overnight. I think that it is better to retain the mathematical meaning of argmax here, and have indmax. We would differ in meaning from python+numpy only, since matlab does not have anything called argmax (unless it is in one of the toolboxes). Matlab does the equivalent of indmax by returning the indices as an optional output argument.

@SamChill
Copy link
Contributor Author

SamChill commented Dec 3, 2012

Viral, I agree. I use numpy/scipy daily in my research so I followed their naming convention, but I think its best to stick to the mathematical meaning when we can.

I have updated this pull request to reflect this discussion. I haven't added arg* just ind*.

@johnmyleswhite
Copy link
Member

I'm all for this now. Could we maybe some @eval magic for these definitions in case we come up with other cases when we'd like to use the ind* notation? I'm thinking of something as simple as:

for (ind, find) in ( (:indmax, :findmax), (:indmin, :findmin))
  @eval begin
    function ($ind)(a::Array) = (($find)(a))[2]
  end
end

@pao
Copy link
Member

pao commented Dec 3, 2012

Updated pull request title.

@SamChill
Copy link
Contributor Author

SamChill commented Dec 3, 2012

@johnmyleswhite I'd vote to hold off generalizing this case with metaprogramming until there exist other find* functions, unless you have some others in mind already.

@johnmyleswhite
Copy link
Member

I don't have any in mind offhand. I've just been on a metaprogramming kick lately.

ViralBShah added a commit that referenced this pull request Dec 6, 2012
@ViralBShah ViralBShah merged commit 64f2c8e into JuliaLang:master Dec 6, 2012
@StefanKarpinski
Copy link
Sponsor Member

Although this has been merged already, I thought I'd bring up here the question of whether minind and maxind is better than indmin and indmax. The argument for this version is that it's a good analogy with argmin and argmax, etc. but I feel like the other does read a little more naturally.

@timholy
Copy link
Sponsor Member

timholy commented Dec 6, 2012

More discoverable by tab-completion, too, which I think is a big bonus.

@johnmyleswhite
Copy link
Member

I'm clearly very biased towards the indmax-style names, but why isn't the tab completion argument a compelling case for making things look like maxfind instead of findmax?

@JeffBezanson
Copy link
Sponsor Member

Could also be found by apropos("max") hopefully.

@timholy
Copy link
Sponsor Member

timholy commented Dec 6, 2012

I'm not saying it is---I believe findmax preceded my own time in the project. I somehow find the argument slightly stronger for maxind than for maxfind, but I can't explain why.

I should also say that I'm fine with whatever gets decided here.

@StefanKarpinski
Copy link
Sponsor Member

It's linguistic. You want to find the "maximum index" (maxind) but you want to "find the maximum" (findmax).

@JeffBezanson
Copy link
Sponsor Member

Hrmmm... I might also say find the "index of the maximum". "maximum index" sounds like length(x)

@johnmyleswhite
Copy link
Member

I agree that it's linguistic, but that doesn't really solve things except by saying that we should use heuristics based on English. After all, Spanish compound words use exactly the opposite ordering strategy: skyscraper becomes scrapes-skies in Spanish.

The tab-completion argument is really a question about what the mental indexing strategy you want people to use should be: is there a family of ind* functions or a family of max* functions? There's clearly no right answer nor any real hope of global consistency since we're not going to start saying maxarg instead of argmax nor are we likely to call things maxfind instead of findmax.

Fun experiment: list as many words as you can that start with "K" and as many words as you can that have "K" as the third letter. That will teach you a lot about how your mind handles memory retrieval based on the start of words.

@dmbates
Copy link
Member

dmbates commented Dec 6, 2012

I agree with Jeff. I would read a name like indmax as "index of the maximum element" and a name like maxind as "maximum allowable index".

@StefanKarpinski
Copy link
Sponsor Member

The "index of the maximum" argument works for me and requires no additional changes. Sold.

@johnmyleswhite:

grep -i '^k' /usr/share/dict/words
grep -i '^..k' /usr/share/dict/words

Oh, wait. You mean without a computer? ;-)

@sth4nth
Copy link

sth4nth commented Dec 31, 2012

I think argmax is valid. Since a vector v is just a map of input index to the value. v[i] is value of the input i (v:i->v[i]), Similarly function is a map of input to the value. f(x) is the value of the input x (f:x->f(x)), so x can also be seen as the index of f(x) in f. The difference is only for vector the input is a integer, for function the input could be arbitrary value. Therefore vector is just a function with restricted domain. That's also why function can been seen as a vector in a functional space.
Therefore argmax works for both cases.
argmax(v) returns i
argmax(f) returns x.

@JeffBezanson
Copy link
Sponsor Member

I happen to agree; argmax(array) can give the index of the largest element, and argmax(func, array) can give the value from the array that maximizes func. Ordering the arguments this way removes any confusion, to me.

@StefanKarpinski
Copy link
Sponsor Member

+1

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

Successfully merging this pull request may close these issues.

9 participants