10

The following function will randomly "sprinkle salt" on a loaded image. For the sake of boosting performance, the conditional statement

uint j = rows == 1 ? 0 : randomRow(generator);

should not be inside the loop.

Instead, I want to define a lambda getJ before the loop as

auto getJ = rows == 1 ? []() {return 0; } : []() {return randomRow(generator); };

However, my code with this lambda does not compile with the following red squiggled text:

enter image description here

Question

How to conditionally define such a lambda?

void salt_(Mat mat, unsigned long long n)
{
    const uchar channels = mat.channels();
    uint cols = mat.cols;
    uint rows = mat.rows;

    if (mat.isContinuous())
    {
        cols *= rows;
        rows = 1;
    }

    default_random_engine generator;
    uniform_int_distribution<uint> randomRow(0, rows - 1);
    uniform_int_distribution<uint> randomCol(0, cols - 1);



    // auto getJ = rows == 1 ? []() {return 0; } : []() {return randomRow(generator); };


    uchar * const data = mat.data;

    for (unsigned long long counter = 0; counter < n; counter++)
    {
        uint i = randomCol(generator);
        uint j = rows == 1 ? 0 : randomRow(generator);
        //uint j = getJ();

        uint index = channels * (cols * j + i);
        for (uchar k = 0; k < channels; k++)
            data[index + k] = 255;
    }
}
  • 3
    "my code with this lambda does not compile" - What is the error you get? – Suma Apr 16 at 12:35
  • You are trying to reference local variable generator without capturing it. If second lambda was capture-free it would compile. – VTT Apr 16 at 12:36
  • 5
    Seems like a premature optimisation. Surely function call overhead would be greater than ternary overhead? – Artyer Apr 16 at 12:40
  • 2
    Is a conditional really a performance issue here? Have you profiled the code? – Jesper Juhl Apr 16 at 12:42
  • 1
    @Artyer Done well, the call to the lambda can be inlined. – Angew Apr 16 at 12:43
10

my code with this lambda does not compile with the following red squiggled text

You cannot use randomRow inside the body of the lambda expression without capturing it beforehand, as the generated closure object needs to have access to it.

Even if you were to use [&randomRow], the code would still fail to compile as every lambda expression produces a closure of unique type, even if the lambda expressions are exactly the same.

You can turn the problem on its head to avoid any overhead and achieve what you want - create a function that takes the lambda you want to invoke:

template <typename F>
void saltImpl(F&& getJ, /* ... */)
{
    uchar * const data = mat.data;

    for (unsigned long long counter = 0; counter < n; counter++)
    {
        uint i = randomCol(generator);
        uint j = rows == 1 ? 0 : randomRow(generator);
        //uint j = getJ();

        uint index = channels * (cols * j + i);
        for (uchar k = 0; k < channels; k++)
            data[index + k] = 255;
    }
}

Usage example:

void salt_(Mat mat, unsigned long long n)
{
    const uchar channels = mat.channels();
    uint cols = mat.cols;
    uint rows = mat.rows;

    if (mat.isContinuous())
    {
        cols *= rows;
        rows = 1;
    }

    default_random_engine generator;
    uniform_int_distribution<uint> randomRow(0, rows - 1);
    uniform_int_distribution<uint> randomCol(0, cols - 1);

    if (rows == 1)
    {
        saltImpl([]{ return 0; }, /* ... */);
    }
    else
    {
        saltImpl([&]{ return randomRow(generator); }, /* ... */)
    }
}
3

Why this fails is because the lambdas are of a different type. That's natural, their operator() have different definitions. Which means you want your following code to work with two different types. And the C++ way of making code work with different types is using templates.

Convert the code using getJ to a function template (it can be local to your implementation file), like this:

template <class G>
void salt_impl_(Mat mat, unsigned long long n, default_random_engine &generator, G getJ)
{
    const uchar channels = mat.channels();
    uint cols = mat.cols;
    uint rows = mat.rows;

    if (mat.isContinuous())
    {
        cols *= rows;
        rows = 1;
    }

    uchar * const data = mat.data;

    uniform_int_distribution<uint> randomCol(0, cols - 1);

    for (unsigned long long counter = 0; counter < n; counter++)
    {
        uint i = randomCol(generator);
        uint j = getJ();

        uint index = channels * (cols * j + i);
        for (uchar k = 0; k < channels; k++)
            data[index + k] = 255;
    }
}


void salt_(Mat mat, unsigned long long n)
{
    const uchar channels = mat.channels();
    uint cols = mat.cols;
    uint rows = mat.rows;

    if (mat.isContinuous())
    {
        cols *= rows;
        rows = 1;
    }

    default_random_engine generator;
    uniform_int_distribution<uint> randomRow(0, rows - 1);

    if (rows == 1)
      salt_impl_(mat, n, generator, []() {return 0; });
    else
      salt_impl_(mat, n, generator, [&]() {return randomRow(generator); });
}

Feel free to reduce the initial-part duplication between the function and the template by passing more parameters, making them members of a class, or something similar.

Also note that the non-trivial lambda must capture the variables which it accesses (randomRow and generator). I did this using the universal by-reference capture [&] in the code above.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.