topk includes NaNs when it should always prefer numbers

[aecolley]

If topk is given a vector of numbers mixed with a few NaNs, it will conditionally include NaNs in the output depending on the ordering of the input vector.

For example, here's an artificial expression which delivers a mixed vector:

mixed = bottomk(3, up) * 0 or bottomk(10, up * (0/0))

Now mixed is a vector with three 0s and seven NaNs. If we use sort to reorder the vector, so that the 0s are at the end, then topk will select NaNs:

topk(5, sort(mixed)) // 5xNan
topk(5, sort_desc(mixed)) // 3x0, 2xNaN

Now, it's an interesting question what topk should do when it doesn't have enough numbers but does have NaNs: should it filter out the NaNs or test them as less-than every number? But I'm pretty sure it should never prefer NaNs over numbers.

Even if you don't agree with that, you surely agree that the sort order of the input shouldn't influence the output of topk.

[matthiasr]

I would expect NaN to be treated as absent in this context, i.e. never include them in topk/bottomk. If there aren't enough things, it should return how ever many there are.

[brian-brazil]

I'd expect NaN to be included if needed - if someone asks for 10 things and there's more than 10 inputs then they should get 10 things. We've had to special case sorting elsewhere due to NaN.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.