Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

integer overflow in int-benchmark #12

Closed
travisdowns opened this issue Jun 11, 2020 · 4 comments
Closed

integer overflow in int-benchmark #12

travisdowns opened this issue Jun 11, 2020 · 4 comments

Comments

@travisdowns
Copy link

travisdowns commented Jun 11, 2020

In int-benchmark the following code is used for generating the random distribution:

    std::generate(values.begin(), values.end(), []() {
      int scale = std::rand() / 100 + 1;
      return (std::rand() * std::rand()) / scale;
    });

std::rand() * std::rand() may or may not suffer from integer overflow depending on RAND_MAX. On Linux where RAND_MAX is usually 2^31-1, overflow almost always occurs (> 99.99% of the time) and this results in very different results (50% of values are negative) to windows where RAND_MAX is 2^15 (all positive values and smaller values in general). Not sure about OSX.

I guess this is not intentional. Ideally, once would use the same RNG on every platform. I don't know if the 50% negative values is a "feature" or not.

@vitaut
Copy link
Contributor

vitaut commented Jun 11, 2020

This code originates from karma generate benchmark (https://www.boost.org/doc/libs/1_52_0/libs/spirit/optimization/karma/int_generator.cpp). I think 50% negative is a feature and a PR to make the benchmark more consistent across platforms would be welcome.

@vitaut
Copy link
Contributor

vitaut commented Jun 13, 2020

Fixed in 7ccec77. Thanks for reporting!

@vitaut vitaut closed this as completed Jun 13, 2020
@travisdowns
Copy link
Author

travisdowns commented Jun 13, 2020

Thanks @vitaut and sorry on the delay.

I kind of got blocked when thinking about a PR because it seemed like making it deterministic across platforms would also be a good goal and <random> is kind of awkward since IIRC it has no good cross-plat RNGs, so I was like bleh.

@vitaut
Copy link
Contributor

vitaut commented Jun 13, 2020

deterministic across platforms would also be a good goal

That would be good and PR is still welcome =). However, the current version shouldn't be too bad - the inputs are not identical but at least distribution is similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants