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

<random>: subtract_with_carry_engine textual representation is incorrect #442

Closed
BillyONeal opened this issue Jan 23, 2020 · 2 comments · Fixed by #2088
Closed

<random>: subtract_with_carry_engine textual representation is incorrect #442

BillyONeal opened this issue Jan 23, 2020 · 2 comments · Fixed by #2088
Labels
bug Something isn't working fixed Something works now, yay!

Comments

@BillyONeal
Copy link
Member

BillyONeal commented Jan 23, 2020

Describe the bug
In our subtract_with_carry_engine, if the result_type is 64 bits, we are still emitting 32 bit values. The standard says:

The textual representation consists of the values of Xi−r, . . . ,Xi−1, in that order, followed by c.

Command-line test case
STL version (git commit or Visual Studio version):

C:\Users\bion\Desktop>type test.cpp
#include <assert.h>
#include <iostream>
#include <random>
#include <sstream>
#include <string_view>

using namespace std::string_view_literals;

constexpr auto expected =
    "3058990831114416762 8271088423883502182 8539773296756973456 "
    "7389019366887995718 570256334847185029 3716502990755565555 "
    "1524729574385026041 3137027355700498078 462318917886328761 "
    "4995455229142172915 4449332696079976531 1322748335426821320 "
    "1481389978760661022 8563059522958733270 3849224045900943705 "
    "4972782340528344279 5835416012924508119 1706253864337811739 "
    "5350051069717426128 2939050575112930398 2997396533930240412 "
    "2702983745895389727 6734488758036272684 2336722746921380976 0 0"sv;

int main() {
  std::subtract_with_carry_engine<unsigned long long, 64, 10, 24> ull_swc;
  ull_swc.seed(0x12341234'00000000ULL);
  std::ostringstream oss;
  oss << ull_swc;
  const auto actual = std::move(oss).str();
  std::cout << "Expected: " << expected << "\n"sv;
  std::cout << "Actual: " << actual << "\n"sv;
  assert(expected == actual);
}

C:\Users\bion\Desktop>cl /EHsc /W4 /WX /std:c++latest /nologo .\test.cpp
test.cpp

C:\Users\bion\Desktop>.\test.exe
Expected: 3058990831114416762 8271088423883502182 8539773296756973456 7389019366887995718 570256334847185029 3716502990755565555 1524729574385026041 3137027355700498078 462318917886328761 4995455229142172915 4449332696079976531 1322748335426821320 1481389978760661022 8563059522958733270 3849224045900943705 4972782340528344279 5835416012924508119 1706253864337811739 5350051069717426128 2939050575112930398 2997396533930240412 2702983745895389727 6734488758036272684 2336722746921380976 0 0
Actual: 712226804 599814778 1925762841 1936454246 1988320913 1469112208 1720390135 701970758 132773149 2105249925 865315783 2057932787 355003768 868254713 730396098 1664487070 107642011 965656505 1163095056 1482884339 1035940995 1969277011 307976346 1415240904 344912982 1104824350 1993742660 1618685910 896217312 751915353 1157816113 411503831 1358663666 1191040983 397268185 2021533979 1245655834 615821264 684301037 579044446 697885764 1206266268 629337445 1472191007 1567995352 916264492 544060661 886238320 0
Assertion failed: expected == actual, file .\test.cpp, line 27

Also tracked by DevCom-249085 and Microsoft-internal VSO-104809 / AB#104809.

@BillyONeal BillyONeal added the bug Something isn't working label Jan 23, 2020
@BillyONeal

This comment has been minimized.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jan 23, 2020

Second test case:

C:\Users\bion\Desktop>type test.cpp
#include <iostream>
#include <random>
#include <sstream>

template <typename RND> void check(const char *name) {
  RND g1, g2;
  std::cout << std::boolalpha << name << ": " << (g1 == g2);
  std::stringstream s;
  s << g1;
  s >> g1;
  std::cout << " " << (g1 == g2) << std::endl;
}

int main() {
  check<std::ranlux48>("ranlux48");
  check<std::ranlux24>("ranlux24");
  check<std::mt19937>("mt19937");
}

C:\Users\bion\Desktop>cl /EHsc /W4 /WX /std:c++latest /nologo .\test.cpp
test.cpp

C:\Users\bion\Desktop>.\test.exe
ranlux48: true false
ranlux24: true true
mt19937: true true

C:\Users\bion\Desktop>

Expected behavior

ranlux48: true true 
ranlux24: true true 
mt19937: true true

This is the same issue because ranlux48 uses a 64-bit subtract_with_carry_engine:

using ranlux48_base = subtract_with_carry_engine<unsigned long long, 48, 5, 12>;

using ranlux48 = discard_block_engine<ranlux48_base, 389, 11>;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants