Subject: Re: [boost] Add an adapter to cover fast bit functions
From: Evgeny Shulgin [Izaron] (izaronplatz_at_[hidden])
Date: 2018-08-27 23:15:03
Thank you for your reply! It really helped me a lot.
> - Please don't #include <iostream> in headers, if you can avoid it.
My bad, I forgot to remove this huge unused header, I should've used a tool
> - You're using C++14 constexpr, so BOOST_CXX14_CONSTEXPR
> would be the correct macro.
Thanks, I missed the fact that in C++11 it is correct only when the function
has exactly one return
> - You might need to think about whether/how to support
> signed integers. In particular, I'm thinking about
> swap and rotate.
I guess "rotate" should not affect the sign bit. If I remember correctly,
there are commands in Assembler to rotate signed numbers.
"Swap" works with whole bytes and will implicitly take a number as an
unsigned. But it's a theme to discuss, as for other functions.
It's good to check signed types in the benchmark as well.
> - For unit tests, it's better to use a prng instead of random_device.
> non-deterministic tests make it more difficult to reproduce
> test failures.
I'll change it, sounds good.
> - It's better to separate benchmarks from unit tests. In particular,
> storing the elements in a vector is necessary for benchmarking,
> since you don't want to benchmark the generation of the
> values, but for tests, it artificially limits the maximum
> number of values you can check.
Values generation isn't benchmarked, only working time of functions.
I was thinking of separating the whole stuff in this way:
- a single unit test verifies the correctness of the algorithm comparing it
with naive functions on generated values
- the benchmark file isn't a test really, but could be used in CI to produce
information about performance
if it's possible (though I guess it's be possible)
Also, IMO, there should be a tiny single header for _every_ function, since
a large file with a ton of monotonic specializations
is pretty hard to support.
> - For char and short, testing every possible input is easy.
> You can do int, too, if you're willing to wait a bit for
> it to finish.
I think it's too much to check every int32 value, because it takes really a
lot of time for almost nothing. We can use many generated values
with corner cases like the maximal/minimal value of each type. The idea is
> Or Boost.Integer.
I explored this, it looks more appropriate to insert fresh code. I hope
someone would take a look at PR if I make the code look
really much better and send it.
-- Sent from: http://boost.2283326.n4.nabble.com/Boost-Dev-f2600599.html
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk