10

Let us say I've got a collection of items and a score function on them:

struct Item { /* some data */ };
std::vector<Item> items;
double score(Item);

I'd like to find the item from that collection whose score is the lowest. An easy way to write this is:

const auto argmin = std::min_element(begin(items), end(items), [](Item a, Item b) {
    return score(a) < score(b);
});

But if score is a heavy-to-compute function, the fact that std::min_element actually calls it multiple times on some items may be worrying. And this is expected because the compiler cannot guess score is a pure function.

How could I find argmin but with score being called only once per item? Memoization is one possibility, anything else?

My objective is to write a code snippet which is easy to read, in a dream world as obvious as calling std::min_element on the collection is.

YSC
  • 38,212
  • 9
  • 96
  • 149
  • 1
    You only need to store the most up to date minimal score, as far as I see it. Then you only need to call `score` once. Though it's kinda hackish, to be frank. – StoryTeller - Unslander Monica Jan 18 '18 at 13:52
  • @StoryTeller I know how I could write a bunch of lines to do that, but I'd like to do it in a way allowing a quick reader to instantly understand what's going on. – YSC Jan 18 '18 at 14:08
  • you need to evaluate `score(item)` at least once for each item, so just do that and then find the minimum in the results (hope noone saw my completely confused previous comment ;) – 463035818_is_not_an_ai Jan 18 '18 at 14:09
  • 4
    If the vector is not too big, you can use `std::transform` to store all scores first, then apply `std::min_element`. – llllllllll Jan 18 '18 at 14:09
  • 1
    @liliscent It would find the minimum _score_, not the _item_ minimizing `score` ;) – YSC Jan 18 '18 at 14:14
  • 2
    But then `std::distance` from `begin` of the new vector to the `std::min_element` would give you an index which holds the item with the minimum score in the original `vector` – Fureeish Jan 18 '18 at 14:16
  • how about writing a function like in John Zwincks answer, then calling that function? – Thijs Steel Jan 18 '18 at 14:16
  • @YSC It will return you the index `i` of minimizer, thus `items[i]` is what you want. – llllllllll Jan 18 '18 at 14:17
  • 1
    @liliscent (a)Fureeish Yes that's right. It's worth an answer. – YSC Jan 18 '18 at 14:24
  • `the compiler cannot guess score is a pure function` what about C++14 `constexpr`? – greybeard Jan 18 '18 at 16:17
  • @greybeard I have no idea. This is an interesting question, though its answer might be compiler-dependant: can a call to a `constexpr` pure function can be optimized away? – YSC Jan 18 '18 at 16:27

3 Answers3

3

Here's a function that does what you want--even going beyond the intuitive "call score exactly once per element" by realizing that there's nothing smaller than negative infinity!

const Item* smallest(const std::vector<Item>& items)
{
    double min_score = items.empty() ? NAN : INFINITY;
    const Item* min_item = items.empty() ? nullptr : &*begin(items);
    for (const auto& item : items) {
        double item_score = score(item);
        if (item_score < min_score) {
            min_score = item_score;
            min_item = &item;
            if (item_score == -INFINITY) {
                break;
            }
        }
    }
    return min_item;
}
John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • I've updated my question which was not clear enough. I'm sorry but I'd prefer a code snippet which let the code as easy to understand as `std::min_element` is. – YSC Jan 18 '18 at 14:13
  • @YSC: I've updated my answer to make it a full-fledged function which at the call site will be even easier to understand than `std::min_element`. Just say `auto argmin = smallest(items);` and you're done. Of course, it can be made fully generic taking `score` as a functor and `items` as a template type if you want. – John Zwinck Jan 18 '18 at 14:17
  • yeah.. i'm still hoping for some functions from `` to get something similar with less writing (I'm lazy). But if I don't get anything I'll accept your answer although we learn nothing from it (don't take it personally, i'm not questioning your capability, it's just that if this answer is the best one could give, the question itself is meaningless). – YSC Jan 18 '18 at 14:23
  • @YSC: I wrote all the code for you, I don't understand how laziness might play a part in it. For you to now say "we learn nothing" is offensive. – John Zwinck Jan 18 '18 at 14:26
  • 1
    I'm sorry it is not meant to be. I value your input, really. But if this is the best that can be, my question is worth nothing. About laziness, I'm being metaphoric here: i'm just saying I like my code as succinct as possible. – YSC Jan 18 '18 at 14:28
  • @YSC , _if this answer is the best one could give, the question itself is meaningless_ is Controversial! .. , because an answer using _functions from _ would probably be just writing how to use that function (boring! and meaningless) .. without showing the _technical_ brilliance .. the leverage of this _technical_ answer being a guide to choose, which can't be considered meaningless :) – Mohammad Kanan Jan 18 '18 at 15:33
  • 1
    @MohammadKanan _meaningless to me_* then^^ I think there is no technical brilliance in finding a minimum in a discrete, finite set. The brilliance could, on the other hand, lie on the simplicity and readability of the code doing so. – YSC Jan 18 '18 at 15:36
  • 1
    @YSC, perception here is different. the condition you put in the question was the real question _calling score() multiple times on some items may be worrying_ .. so this beyond iterating a a discrete finite set .. I am saying the question was NOT meaningless NOR the answer ... knowing that a function from could do it is just an add on :) – Mohammad Kanan Jan 18 '18 at 15:48
  • 2
    Just for information, I did upvote this answer. I think **this is a perfectly valid answer**. It just is not what I expected and I updated my question _after_ John answered. I'm sorry if I mislead anyone thinking otherwise. – YSC Jan 18 '18 at 15:48
  • (Put the check for `-INFINITY` in "the *new `min_item` found*-block". I'm out of practice regarding const correctness: is that saying *the vector shall not be modified*, without declaring same about each item (but the return value)?) – greybeard Jan 19 '18 at 00:04
  • @greybeard I agree, `smallest` should return a (non-const) `Item*`. – YSC Jan 19 '18 at 15:56
  • @YSC: No, because a `const vector<>&` gives only `const_iterators`, and we are effectively returning a `const Item*` "iterator" into that vector. If it were `const vector&` then yes, we could return non-const `Item*`. – John Zwinck Jan 21 '18 at 12:25
  • @john yes, that's what I meant. – YSC Jan 21 '18 at 21:57
3

As I commented above, if the vector is not too big, you can use std::transform to store all scores first, then apply std::min_element.

However, if you want to take benefit of "lazy evaluation", and still want to use C++'s STL, there are some tricks to work it out.

The point is std::accumulate can be regarded as a general reduce or fold operation (like foldl in haskell). With C++17's syntax sugar for std::tuple, we can write something like:

    auto [min_ind, _, min_value] = std::accumulate(items.begin(), items.end(),
        std::make_tuple(-1LU, 0LU, std::numeric_limits<double>::max()),
        [] (std::tuple<std::size_t, std::size_t, double> accu, const Item &s) {
            // up to this point, the index of min, the current index, and the last minimal value
            auto [min_ind, cur_ind, prev_min] = accu;
            double r = score(s);
            if ( r < prev_min ) {
                return std::make_tuple(cur_ind, cur_ind + 1, r);
            } else {
                return std::make_tuple(min_ind, cur_ind + 1, prev_min);
            }
    });
llllllllll
  • 16,169
  • 4
  • 31
  • 54
  • I simultaneously transformed your suggestion into an answer. i'm happy we have different reading. Give me a sec to compare them :) – YSC Jan 18 '18 at 15:26
  • It is indeed really close to what I would have written in a functional language as Haskell or OCaml are. – YSC Jan 18 '18 at 15:30
1

As suggested bu user @liliscent, one could:

  1. generate a collection of precalculated scores,
  2. find the minimum score from it,
  3. and infer the position of the minimizing item from the position of the minimum score.

This is my reading of their suggestion:

template<class InputIt, class Scoring>
auto argmin(InputIt first, InputIt last, Scoring scoring)
{
    using score_type = typename std::result_of_t<Scoring(typename std::iterator_traits<InputIt>::value_type)>;
    std::vector<score_type> scores(std::distance(first, last));
    std::transform(first, last, begin(scores), scoring);
    const auto scoremin = std::min_element(begin(scores), end(scores));
    return first + std::distance(begin(scores), scoremin);
}

With a live demo.

YSC
  • 38,212
  • 9
  • 96
  • 149