5
\$\begingroup\$

I have designed a class bunji::Tensor which is a multi-dimensional array. I have designed it to have a similar interface to a multi-dimensional std::vector, just that the bunji::Tensor constructor takes in an std::vector in its constructor which defines the dimensions, i.e.

bunji::Tensor<double, 3> my_tensor({10, 4, 2});

Creates a 3 dimensional tensor of doubles, with dimensions x=10, y=4, and z=2.

I have made some tests for this tensor using the google/googletest testing framework. My tests are below.

#include "tensor.hpp"

#include <gtest/gtest.h>

void test_manual_1d(const std::size_t x)
{
    bunji::Tensor<int, 1> tensor({x});

    for (std::size_t i = 0; i < x; ++i)
    {
        EXPECT_EQ(tensor[i], 0);
        tensor[i] = i+1;
    }

    for (std::size_t i = 0; i < x; ++i)
    {
        EXPECT_EQ(tensor[i], i+1);
    }
}

void test_manual_2d(const std::size_t x, const std::size_t y)
{
    bunji::Tensor<int, 2> tensor({x, y});

    for (std::size_t i = 0; i < x; ++i)
    {
        for (std::size_t j = 0; j < y; ++j)
        {
            EXPECT_EQ(tensor[i][j], 0);
            tensor[i][j] = (i+1) * (j+1);
        }
    }

    for (std::size_t i = 0; i < x; ++i)
    {
        for (std::size_t j = 0; j < y; ++j)
        {
            EXPECT_EQ(tensor[i][j], (i+1) * (j+1));
        }
    }
}

void test_manual_3d(const std::size_t x, const std::size_t y, const std::size_t z)
{
    bunji::Tensor<int, 3> tensor({x, y, z});

    for (std::size_t i = 0; i < x; ++i)
    {
        for (std::size_t j = 0; j < y; ++j)
        {
            for (std::size_t k = 0; k < z; ++k)
            {
                EXPECT_EQ(tensor[i][j][k], 0);
                tensor[i][j][k] = (i+1) * (j+1) * (k+1);
            }
        }
    }

    for (std::size_t i = 0; i < x; ++i)
    {
        for (std::size_t j = 0; j < y; ++j)
        {
            for (std::size_t k = 0; k < z; ++k)
            {
                EXPECT_EQ(tensor[i][j][k], (i+1) * (j+1) * (k+1));
            }
        }
    }
}

void test_manual_4d(const std::size_t x, const std::size_t y, const std::size_t z, const std::size_t v)
{
    bunji::Tensor<int, 4> tensor({x, y, z, v});

    for (std::size_t i = 0; i < x; ++i)
    {
        for (std::size_t j = 0; j < y; ++j)
        {
            for (std::size_t k = 0; k < z; ++k)
            {
                for (std::size_t l = 0; l < v; ++l)
                {
                    EXPECT_EQ(tensor[i][j][k][l], 0);
                    tensor[i][j][k][l] = (i+1) * (j+1) * (k+1) * (l+1);
                }
            }
        }
    }

    for (std::size_t i = 0; i < x; ++i)
    {
        for (std::size_t j = 0; j < y; ++j)
        {
            for (std::size_t k = 0; k < z; ++k)
            {
                for (std::size_t l = 0; l < v; ++l)
                {
                    EXPECT_EQ(tensor[i][j][k][l], (i+1) * (j+1) * (k+1) * (l+1));
                }
            }
        }
    }
}

template<std::size_t Dimensions, class Callable>
void nd_for_loop(std::size_t begin, std::size_t end, Callable &&c)
{
    for(size_t i = begin; i != end; ++i)
    {
        if constexpr(Dimensions == 1)
        {
            c(i);
        }
        else
        {
            auto bind_argument = [i, &c](auto... args)
            {
                c(i, args...);
            };
            nd_for_loop<Dimensions-1>(begin, end, bind_argument);
        }
    }
}

TEST(tensor, tensor_manual)
{
    nd_for_loop<1>(0, 8500, test_manual_1d);
    nd_for_loop<2>(0, 100, test_manual_2d);
    nd_for_loop<3>(0, 24, test_manual_3d);
    nd_for_loop<4>(0, 12, test_manual_4d);
}

int main(int argc, char **argv)
{
    testing::InitGoogleTest(&argc, argv);
    return RUN_ALL_TESTS();
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Wanna share tensor.hpp? \$\endgroup\$
    – coderodde
    Sep 9, 2022 at 12:14
  • \$\begingroup\$ @coderodde Sure, the code is on my github repository, I didnt share it initially as my intention was for it to be similar in use to a multidimensional std::vector, and it isn't what I would like to be reviewed. \$\endgroup\$ Sep 9, 2022 at 12:18

2 Answers 2

5
\$\begingroup\$

The biggest issue with your test suite is the fact that you had to manually write four versions to handle tensors of up to four dimensions. You should be able to write generic code to test arbitrary-dimension tensors. Ideally, all test_manual_*() functions should be replaced with:

template<typename... Dims>
void test(const Dims&... dims) {
    bunji::Tensor<int, sizeof...(Dims)> tensor({dims...});
    ...
}

Now the question is: how to implement an arbitrarily-nested for-loop in a generic way? The solution is to store the indices in a std::vector or std::array, and treat this as a multidimensional index. Then you can create a function to increment this:

template<std::size_t N>
static bool next(std::array<std::size_t, N>& indices, const std::array<std::size_t, N>& dims) {
    for (std::size_t i = 0; i < N; ++i) {
        if (++indices[i] != dims[i])
            return true;
        indices[i] = 0;
    }
    return false;
}

The way you use it is like so:

template<typename... Dims>
static void test(const Dims&... dims) {
    constexpr std::size_t N = sizeof...(Dims);
    bunji::Tensor<int, N> tensor({dims...});

    std::array<std::size_t, N> indices = {};
    std::array<std::size_t, N> dims_arr = {dims...};

    do {
        ...
    } while (next<N>(indices, dims_arr));
}

The next issue is how to generically apply [] multiple times. A recursive template function can be used here, similar to how nd_for_loop() works:

template<std::size_t N, std::size_t I = N - 1, typename T>
static auto& get(T& tensor, const std::array<std::size_t, N>& indices) {
    if constexpr (I == 0) {
        return tensor[indices[0]];
    } else {
        return get<N, I - 1>(tensor[indices[I]], indices);
    }
}

Then inside the do-while-loop above you can write:

auto& value = get<N>(tensor, indices);
EXPECT_EQ(value, 0);
value = std::accumulate(indices.begin(), indices.end(), 1, std::multiplies<int>());

There are some variations possible on this theme regarding how exactly you pass the dimensions to the various functions. Maybe pass a std::array to test() as well, instead of using a variable number of arguments? That would simplify nd_for_loop().

\$\endgroup\$
2
  • 2
    \$\begingroup\$ Oh, all that is ending up with test code of sufficient complexity that it itself needs tests! An issue I was considering mentioning if I do a review of my own. Quis probatiet ipsos probates?, as one might ask... \$\endgroup\$ Sep 9, 2022 at 18:17
  • \$\begingroup\$ Thanks, that is a really nice implementation. It works perfectly once the bugs are ironed out! \$\endgroup\$ Sep 9, 2022 at 18:28
7
\$\begingroup\$

Simple things first - GTest provides a main() that you can use - build and link with gtest-main.cc.

I get a bunch of compiler warnings from the header, but that's not in scope for this review. But do make sure you've enabled a good set of warnings.

Moving onto the tests themselves, they are quite clever. That's not something we want in test code. Really, we want something that's so simple that the test is obviously correct. If it's not obviously correct, then the test itself needs to be tested (and that's sometimes what we need to do - demonstrate that it correctly fails when given a broken implementation).

One thing the tests demonstrate is how useable (or not) the interface is. For example, we don't have constructor deduction (and can't currently, but could make a step to that by having a constructor that accepts std::array<std::size_t, N>.

I generally begin a test suite by making sure the error cases are handled well. So I'd start with:

using bunji::Tensor;

TEST(tensor, zero_dimension)
{
    Tensor<int, 0> t(std::vector<std::size_t>{});
}

TEST(tensor, negative_dimensions)
{
    Tensor<int, -1> t(std::vector<std::size_t>{});
}

Ideally, neither of these would compile successfully. However, the first one not only compiles, but also passes. And the second compiles, but crashes the test harness (rather than e.g. throwing an exception).

Once we have these failing, we could use the detection idiom to turn them into real does-not-compile tests.

The tests we have only demonstrate that values written through the [] interface can be read back successfully via the same interface. That's only the beginning of a functional test. I would prefer a test that wrote values and then compared against a Tensor constructed inline. And then a test that constructs a Tensor, then reads the values individually. It's worth creating a custom GTest comparator, rather than simply using operator=, so you can show how the actual and expected differ, rather than merely the fact that they do.

We have tested only int as the value type - I'd like to see some other arithmetic types (e.g. std::complex) and perhaps std::string or similar, too.

And there's much more control-flow than I like to see in tests. Unless the tests have tests of their own, we should aim to minimise branches (including loops) in our test code. We want the tests to be so simple that they are obviously correct, unless we test the tests (e.g. with mutation testing).

\$\endgroup\$
6
  • \$\begingroup\$ I appreciate that this is almost the opposite of G. Sliepen's answer, and that's to some extent intentional, as it can be useful to consider a spectrum of viewpoints. I place a low value on cleverness in tests, and want them to be nicely obvious, so that I know my (clever) product code works well! \$\endgroup\$ Sep 9, 2022 at 19:04
  • \$\begingroup\$ I appreciate your view on things as well :) As for tensors with zero dimensions, I would keep those, and ensure they behave exactly the same as scalars (see the examples in the Wikipedia article about tensors). \$\endgroup\$
    – G. Sliepen
    Sep 9, 2022 at 19:31
  • 1
    \$\begingroup\$ Yes, that makes sense, @G.Sliepen. But negative dimensions are right out (I don't see why a signed type was used for the dimension count). \$\endgroup\$ Sep 9, 2022 at 19:34
  • \$\begingroup\$ Thanks for the response! I will look into using the detection idiom, and I will make some more dumbed down tests. \$\endgroup\$ Sep 10, 2022 at 8:36
  • \$\begingroup\$ I don’t think the detection algorithm is going to help you with does-not-compile tests. If bunji::Tensor<T, N> will fail to compile when N is −1, I’m pretty sure there’s just no way to detect that in standard C++ (and have it actually compile). Try it yourself. It’s likely that your only option is to resort to build system shenanigans. \$\endgroup\$
    – indi
    Sep 10, 2022 at 21:57

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.