This is a follow-up question for An Updated Multi-dimensional Image Data Structure with Variadic Template Functions in C++ and rand Template Function Implementation for Image in C++. I implemented randi
template function which usage is like Matlab's randi
function in this post.
Description
X = TinyDIP::randi(imax)
returns a pseudorandom scalar integer between 1 and imax.X = TinyDIP::randi(std::make_pair(imin, imax))
returns a pseudorandom scalar integer between imin and imax.X = TinyDIP::randi(imax, n)
returns an n-by-n matrix of pseudorandom integers between 1 and imax.X = TinyDIP::randi(std::make_pair(imin, imax), n)
returns an n-by-n matrix of pseudorandom integers between imin and imax.X = TinyDIP::randi(imax, sz1, ..., szN)
returns an sz1-by-...-by-szN Image where sz1,...,szN indicates the size of each dimension. For example,TinyDIP::randi(10,3,4)
returns a 3-by-4 Image of pseudorandom integers between 1 and 10.X = TinyDIP::randi(std::make_pair(imin, imax), sz1, ..., szN)
returns an sz1-by-...-by-szN Image where sz1,...,szN indicates the size of each dimension. For example,TinyDIP::randi(std::make_pair(10,100),3,4)
returns a 3-by-4 Image of pseudorandom integers between 10 and 100.
The experimental implementation
randi
template function implementationnamespace TinyDIP { // randi template function implementation template<std::integral ElementT = int, typename Urbg, std::same_as<std::size_t>... Sizes> requires std::uniform_random_bit_generator<std::remove_reference_t<Urbg>> constexpr static auto randi(Urbg&& urbg, std::pair<ElementT, ElementT> min_and_max, Sizes... sizes) { if constexpr (sizeof...(Sizes) == 1) { return randi(std::forward<Urbg>(urbg), min_and_max, sizes..., sizes...); } else { std::vector<ElementT> image_data((... * sizes)); auto dist = std::uniform_int_distribution<ElementT>{ min_and_max.first, min_and_max.second }; std::ranges::generate(image_data, [&dist, &urbg]() { return dist(urbg); }); return Image<ElementT>{std::move(image_data), sizes...}; } } // randi template function implementation template<std::integral ElementT = int, std::same_as<std::size_t>... Size> inline auto randi(std::pair<ElementT, ElementT> min_and_max, Size... size) { return randi<ElementT>(std::mt19937{std::random_device{}()}, min_and_max, size...); } // randi template function implementation template<std::integral ElementT = int, std::same_as<std::size_t>... Size> inline auto randi(ElementT max, Size... size) { return randi<ElementT>(std::mt19937{ std::random_device{}() }, std::make_pair(1, max), size...); } // randi template function implementation template<std::integral ElementT = int, typename Urbg> requires std::uniform_random_bit_generator<std::remove_reference_t<Urbg>> constexpr auto randi(Urbg&& urbg, std::pair<ElementT, ElementT> min_and_max) -> ElementT { auto dist = std::uniform_int_distribution<ElementT>{ min_and_max.first, min_and_max.second }; return dist(urbg); } // randi template function implementation template<std::integral ElementT = int, typename Urbg> requires std::uniform_random_bit_generator<std::remove_reference_t<Urbg>> constexpr auto randi(Urbg&& urbg, ElementT max) -> ElementT { return randi(urbg, std::make_pair(static_cast<ElementT>(1), max)); } // randi template function implementation template<std::integral ElementT = int> inline auto randi(ElementT max) { return randi<ElementT>(std::mt19937{std::random_device{}()}, max); } }
The usage of randi
template function:
/* Developed by Jimmy Hu */
#include <chrono>
#include "../base_types.h"
#include "../basic_functions.h"
#include "../image.h"
#include "../image_operations.h"
#include "../timer.h"
void randiFunctionTest(
const std::size_t sizex = 3,
const std::size_t sizey = 2)
{
// Single Random Integer
auto randi_output1 = TinyDIP::randi(10);
std::cout << "Single Random Integer: " << randi_output1 << '\n';
// Single Random Integer with Specified Range
auto randi_output2 = TinyDIP::randi(std::make_pair(10, 100));
std::cout << "Single Random Integer with Specified Range: " << randi_output2 << '\n';
// Square Matrix of Random Integers
auto randi_output3 = TinyDIP::randi(10, sizex);
std::cout << "Square Matrix of Random Integers: \n";
randi_output3.print();
// sizex-by-sizey image of pseudorandom integers
auto randi_output4 = TinyDIP::randi(10, sizex, sizey);
std::cout << "sizex-by-sizey image of pseudorandom integers: \n";
randi_output4.print();
// Square Matrix of Random Integers with Specified Range
auto randi_output5 = TinyDIP::randi(std::make_pair(10, 100), sizex);
std::cout << "Square Matrix of Random Integers with Specified Range: \n";
randi_output5.print();
// sizex-by-sizey image of pseudorandom integers with specified range
auto randi_output6 = TinyDIP::randi(std::make_pair(10, 100), sizex, sizey);
std::cout << "sizex-by-sizey image of pseudorandom integers with specified range: \n";
randi_output6.print();
return;
}
int main()
{
TinyDIP::Timer timer1;
randiFunctionTest();
return EXIT_SUCCESS;
}
The output of the test code above:
Single Random Integer: 4
Single Random Integer with Specified Range: 23
Square Matrix of Random Integers:
9 7 5
9 5 8
3 2 8
sizex-by-sizey image of pseudorandom integers:
5 8 2
5 1 9
Square Matrix of Random Integers with Specified Range:
51 45 96
25 83 93
89 29 11
sizex-by-sizey image of pseudorandom integers with specified range:
85 91 59
58 98 44
Computation finished at Sat Mar 8 20:21:52 2025
elapsed time: 0.0265732 seconds.
All suggestions are welcome.
The summary information:
Which question it is a follow-up to?
An Updated Multi-dimensional Image Data Structure with Variadic Template Functions in C++ and
What changes has been made in the code since last question?
I implemented
randi
template function in this post.Why a new review is being asked for?
Please review the implementation of
randi
template function and its tests.
1 Answer 1
The interface is confusing
There are many overloads of the randi()
function, and some are doing things that, at least for me who never used MATLAB, are unexpected. The most surprising thing is that sizes
is a parameter pack of sizes for a variable number of dimensions, and if you specify only one size, I would expect to get a one-dimensional result. However, it gives you a two-dimensional result. There seems to be no way to get a one-dimensional result from your randi()
, even though one-dimensional images are sometimes useful.
The other surprising thing is that if you ask for a zero-dimensional result, you get an ElementT
as a result instead of an Image<ElementT>
. A zero-dimensional Image
is possible to construct with TinyDIP with some effort, but if you don't want that, I'd rather have it not return any valid result than something other than an Image
.
Since this function is constructing a new Image
, it would be nice if it follows exactly the same semantics as the constructor of Image
when it comes to sizes.
Split it up into simpler functions
Even though generating an image filled with random values doesn't sound very complex, randi()
is doing several things: it constructs an image, creates a random number generator, and fills the image with those numbers. Since each of those tasks can be done in different ways, you get this explosion in function overloads, in order to cater for all the possible ways you could do each sub-task. And you even forgot some: what if you want to specify the size of the image using a std::vector<std::size_t>
? 3 more overloads! What if you want to add the equivalent of MATLAB's rand
and randn
functions? Multiply everything by 3!
Consider creating a function that can create an Image
using any generator function:
template<typename F, std::same_as<std::size_t>... Sizes>
requires std::invocable<F&>
auto generate(F gen, Sizes... sizes) {
using ElementT = std::invoke_result_t<F>;
Image<ElementT> image(sizes...);
std::ranges::generate(image.image_data, gen);
return image;
}
You can consider adding an overload for passing sizes as a std::vector
.
This wouldn't even be necessary if there was a std::views::generate
, because then you could just have constructed an image from any function generating values:
auto image = TinyDIP::Image(
std::views::generate([]{ return dist(gen); },
sizex, sizey);
You could still create a randi()
that works on top of generate()
,
Now you have a nice, generic, reusable function to create images using a generator function. Of course, this one uses the private member variable .image_data
, so either you have to make it a friend
or add a public member function to get mutable access to the underlying storage of an Image
.
Now you can write randi()
on top of that, which can then just pass forward all the parameters related to the desired size to generate()
:
template<std::integral ElementT, typename... Args>
auto randi(ElementT max, Args... args) {
auto urbg = std::mt19937{std::random_device{}()};
auto dist = std::uniform_int_distribution<ElementT> dist(1, max);
return generate([&]{ return dist(urbg); }, args...);
}
You can add overloads for passing a minimum/maximum pair and/or an explicit random number generator, but you don't have to worry about a combinatorial explosion of overloads because of the size arguments.
Avoid temporary image_data
You create a temporary vector image_data
, generate random values into that vector, and then move it into a newly constructed Image
. At least, that's what it looks like, but it turns out your constructor makes a copy anyway:
template<std::same_as<std::size_t>... Sizes>
Image(std::vector<ElementT>&& input, Sizes... sizes):
size{sizes...}, image_data(begin(input), end(input))
This should have been:
template<std::same_as<std::size_t>... Sizes>
Image(std::vector<ElementT>&& input, Sizes... sizes):
size{sizes...}, image_data(std::move(input))
But, even better that moving is not having to move at all. Just create an Image
of the desired size, then generate the random values into the storage of that image.
-
2\$\begingroup\$ "with MATLAB that is possible (if you provide a vector of sizes, and that vector itself has size 1)." No, it’s not. MATLAB doesn’t have 1D arrays. Every array has at least two dimensions. You can create an array of 100x1, but not a 1D array. \$\endgroup\$Cris Luengo– Cris Luengo2025年03月08日 20:32:51 +00:00Commented Mar 8 at 20:32
std::vector
or something similar across your whole library, then you have a consistent API and have a lot fewer overloads. \$\endgroup\$