Skip to content

<experimental/random> Implement randint #361

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.

Sign up for GitHub

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

Closed
SuperWig wants to merge 16 commits into from
Closed

<experimental/random> Implement randint #361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
451bcc5
<experimental/random> Implement randint
SuperWig Dec 9, 2019
7fcdf93
Fix validation
SuperWig Dec 9, 2019
51d4641
Improve assert message
SuperWig Dec 9, 2019
2c268b5
const arguments and use default argument for reseed()
SuperWig Dec 9, 2019
a452143
Use _RNG_REQUIRE_INTTYPE for type requirment.
SuperWig Dec 9, 2019
b9cc3cd
Add header to __msvc_all_public_headers.hpp
SuperWig Dec 10, 2019
4fa838f
Add _NODISCARD
SuperWig Dec 10, 2019
408a390
split reseed overloads and make reseed() initialise whole state
SuperWig Dec 10, 2019
ecef960
_STD qualify function calls
SuperWig Dec 10, 2019
dc7c56f
Use braces for _Seq
SuperWig Dec 10, 2019
27a1b20
Implement <experimental/algorithm>
SuperWig Dec 10, 2019
a0231a7
Naming and formatting
SuperWig Dec 11, 2019
3c5b1a5
Include <algorithm>
SuperWig Dec 11, 2019
aa6bd67
static_assert default_random_engine == mt19937
SuperWig Dec 11, 2019
5ffe8bf
Revert "Use braces for _Seq"
SuperWig Dec 12, 2019
58b5c96
Properly seed _Engine()
SuperWig Dec 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions stl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,15 @@ set(HEADERS
${CMAKE_CURRENT_LIST_DIR}/inc/deque
${CMAKE_CURRENT_LIST_DIR}/inc/exception
${CMAKE_CURRENT_LIST_DIR}/inc/execution
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/algorithm
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/coroutine
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/deque
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/filesystem
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/forward_list
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/generator
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/list
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/map
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/random
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/resumable
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/set
${CMAKE_CURRENT_LIST_DIR}/inc/experimental/string
Expand Down
2 changes: 2 additions & 0 deletions stl/inc/__msvc_all_public_headers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,13 @@
#include <cuchar>
#include <cwchar>
#include <cwctype>
#include <experimental/algorithm>
#include <experimental/deque>
#include <experimental/filesystem>
#include <experimental/forward_list>
#include <experimental/list>
#include <experimental/map>
#include <experimental/random>
#include <experimental/set>
#include <experimental/string>
#include <experimental/unordered_map>
Expand Down
46 changes: 46 additions & 0 deletions stl/inc/experimental/algorithm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// algorithm experimental header

// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#pragma once
#ifndef _EXPERIMENTAL_ALGORITHM_
#define _EXPERIMENTAL_ALGORITHM_
#include <yvals_core.h>
#if _STL_COMPILER_PREPROCESSOR

#include <algorithm>
#include <experimental/random>

#pragma pack(push, _CRT_PACKING)
#pragma warning(push, _STL_WARNING_LEVEL)
#pragma warning(disable : _STL_DISABLED_WARNINGS)
_STL_DISABLE_CLANG_WARNINGS
#pragma push_macro("new")
#undef new

_STD_BEGIN
namespace experimental {
inline namespace fundamentals_v3 {

template <class _PopIt, class _SampleIt, class _Diff>
_SampleIt sample(_PopIt _First, _PopIt _Last, _SampleIt _Dest, _Diff _Count) {
return _STD sample(_First, _Last, _Dest, _Count, _Engine());
}

template <class _RanIt>
void shuffle(_RanIt _First, _RanIt _Last) {
_STD shuffle(_First, _Last, _Engine());
}

} // namespace fundamentals_v3
} // namespace experimental
_STD_END

#pragma pop_macro("new")
_STL_RESTORE_CLANG_WARNINGS
#pragma warning(pop)
#pragma pack(pop)

#endif // _STL_COMPILER_PREPROCESSOR
#endif // _EXPERIMENTAL_ALGORITHM_
68 changes: 68 additions & 0 deletions stl/inc/experimental/random
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// random experimental header

Copy link
Member

@StephanTLavavej StephanTLavavej Dec 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am willing to make a special exception to our non-goal of "Implementing Technical Specifications" on a case-by-case basis for this feature, given how obnoxious the lack of easy random number generation is for beginners, but I am concerned about permanently increasing our support cost, and the lack of progress for this proposal within WG21.

// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#pragma once
#ifndef _EXPERIMENTAL_RANDOM_
#define _EXPERIMENTAL_RANDOM_
#include <yvals_core.h>
#if _STL_COMPILER_PREPROCESSOR

#include <random>

#pragma pack(push, _CRT_PACKING)
#pragma warning(push, _STL_WARNING_LEVEL)
#pragma warning(disable : _STL_DISABLED_WARNINGS)
_STL_DISABLE_CLANG_WARNINGS
#pragma push_macro("new")
#undef new

_STD_BEGIN
namespace experimental {
inline namespace fundamentals_v3 {

const seed_seq _Seed_engine() {
static_assert(
is_same_v<default_random_engine, mt19937>, "This code assumes that default_random_engine is mt19937");
constexpr size_t _Nx = mt19937::state_size;
constexpr size_t _Wx = mt19937::word_size;
static_assert(_Wx % 32 == 0, "mt19937::word_size is not a multiple of 32");
constexpr size_t _Kx = _Wx / 32;

uint32_t _Values[_Nx * _Kx];
random_device _Rd;
_STD generate(_STD begin(_Values), _STD end(_Values), _STD ref(_Rd));
return seed_seq(_STD cbegin(_Values), _STD cend(_Values));
}

_NODISCARD inline default_random_engine& _Engine() {
thread_local default_random_engine _Eng(_Seed_engine());
return _Eng;
}

template <class _IntType>
_NODISCARD _IntType randint(const _IntType _Min, const _IntType _Max) {
_RNG_REQUIRE_INTTYPE(randint(), _IntType);
_STL_ASSERT(_Min <= _Max, "invalid min and max arguments for randint()");
return uniform_int_distribution<_IntType>{_Min, _Max}(_Engine());
Copy link
Contributor

@miscco miscco Dec 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct. You are creating a new uniform_int_distribution with a given range and Call operator() on that temporary. However, the specification speaks from a thread_local instance of a uniform_int_distribution. Now I fail to see how that is possible if there are different calls to randint with different parameters. @BillyONeal might know, at least i remember him talking about rngs.

In any case the initialization of the random number engines/distributions is generally rather costly.So recreating one seems Bad.

Copy link
Contributor Author

@SuperWig SuperWig Dec 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case the initialization of the random number engines/distributions is generally rather costly.So recreating one seems Bad.

AFAIK distributions aren't costly only engines. And the engine being used is a static thread_local (static is implied by thread_local unless I'm mistaken) instance that's returned by reference.

Copy link
Member

@BillyONeal BillyONeal Dec 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, the specification speaks from a thread_local instance of a uniform_int_distribution.

That's not observable because uniform_int_distribution is stateless.

Copy link
Member

@StephanTLavavej StephanTLavavej Dec 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically, in our current implementation, uniform_int_distribution is stateless (with respect to the random bits; of course the limits are stored), but the Standard permits random bits to be retained.

Copy link
Member

@BillyONeal BillyONeal Dec 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the Standard permits random bits to be retained

It does? Huh, TIL. http://eel.is/c++draft/rand.req.dist#tab:rand.req.dist says there are only requirements on operator() if the same URBG is used successively.

Copy link
Contributor

@CaseyCarter CaseyCarter Dec 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://eel.is/c++draft/rand.req.dist#tab:rand.req.dist says there are only requirements on operator() if the same URBG is used successively.

That's imposing the uniformity requirement. If you invoke a distribution repeatedly with different engines, then uniformity isn't guaranteed. Indeed it could not be, since I could invoke the distribution repeatedly with the same engine state.

The hint that distributions are allowed to cache some random bits is the specification (and existence) of reset, which guarantees that "Subsequent uses of d do not depend on values produced by any engine prior to invoking reset." (i.e., any cached bits are flushed).

}

inline void reseed() {
_Engine().seed(_Seed_engine());
}

inline void reseed(const default_random_engine::result_type _Value) {
_Engine().seed(_Value);
}
} // namespace fundamentals_v3
} // namespace experimental
_STD_END

#pragma pop_macro("new")
_STL_RESTORE_CLANG_WARNINGS
#pragma warning(pop)
#pragma pack(pop)

#endif // _STL_COMPILER_PREPROCESSOR
#endif // _EXPERIMENTAL_RANDOM_
2 changes: 2 additions & 0 deletions stl/inc/yvals_core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,8 @@
// EXPERIMENTAL
#define __cpp_lib_experimental_erase_if 201411L
#define __cpp_lib_experimental_filesystem 201406L
#define __cpp_lib_experimental_randint 201511L
#define __cpp_lib_experimental_sample 201402L


#ifdef _RTC_CONVERSION_CHECKS_ENABLED
Expand Down