STL icon indicating copy to clipboard operation
STL copied to clipboard

`<random>`: Replace inaccurate `_XLgamma()` with UCRT `lgamma()`

Open StephanTLavavej opened this issue 1 month ago • 1 comments

Fixes #5577.

Overview

To implement <random>'s poisson_distribution and binomial_distribution, the STL had a "log gamma" function for historical reasons. As the UCRT provides C99 lgamma(), we should just call that. This will increase accuracy and reduce the amount of code we need to maintain.

Accuracy

I used Boost.Math's test coverage to compare UCRT lgamma() to STL _XLgamma(). Boost.Math's own implementation is the best (0 failures). UCRT lgamma() has minor accuracy issues (7 failures). STL _XLgamma() was extremely inaccurate (648 failures).

Click to expand the details of my hacked-up accuracy testing:

I used boost_1_89_0\libs\math\test\test_gamma.cpp and test_gamma.hpp. At the end of test_gamma.cpp, to make it header-only, I followed the docs and added:

#define BOOST_TEST_MODULE header-only multiunit test
#include <boost/test/included/unit_test.hpp>

In test_gamma.hpp I added:

#include <cmath>
#include <random>
float ucrt_lgamma(float val) {
    return std::lgamma(val);
}
double ucrt_lgamma(double val) {
    return std::lgamma(val);
}
long double ucrt_lgamma(long double val) {
    return std::lgamma(val);
}
float stl_xlgamma(float val) {
    return std::_XLgamma(val);
}
double stl_xlgamma(double val) {
    return std::_XLgamma(val);
}
long double stl_xlgamma(long double val) {
    return std::_XLgamma(val);
}

Results:

C:\Temp>cl /EHsc /nologo /W4 /wd4459 /std:c++latest /MTd /Od test_gamma.cpp /I. /I D:\GitHub\DELME\boost_1_89_0\libs\math\test /I D:\GitHub\DELME\boost_1_89_0 /DBOOST_TEST_NO_LIB /DBOOST_MATH_NO_REAL_CONCEPT_TESTS && test_gamma.exe
[...]
*** No errors detected

C:\Temp>cl /EHsc /nologo /W4 /wd4459 /std:c++latest /MTd /Od test_gamma.cpp /I. /I D:\GitHub\DELME\boost_1_89_0\libs\math\test /I D:\GitHub\DELME\boost_1_89_0 /DBOOST_TEST_NO_LIB /DLGAMMA_FUNCTION_TO_TEST=ucrt_lgamma /DBOOST_MATH_NO_REAL_CONCEPT_TESTS && test_gamma.exe
[...]
*** 7 failures are detected in the test module "Master Test Suite"

C:\Temp>cl /EHsc /nologo /W4 /wd4459 /std:c++latest /MTd /Od test_gamma.cpp /I. /I D:\GitHub\DELME\boost_1_89_0\libs\math\test /I D:\GitHub\DELME\boost_1_89_0 /DBOOST_TEST_NO_LIB /DLGAMMA_FUNCTION_TO_TEST=stl_xlgamma /DBOOST_MATH_NO_REAL_CONCEPT_TESTS && test_gamma.exe
[...]
*** 648 failures are detected in the test module "Master Test Suite"

Changes

<random>

Don't declare _XLgamma() anymore. (The all-important _CRTIMP2_PURE will be preserved on the definitions.)

Call _STD lgamma(), which is similarly overloaded for float / double / long double.

xlgamma.cpp

Transfer _CRTIMP2_PURE to the definitions, which will keep them dllexported. This replaces a silly pattern of "declare with _CRTIMP2_PURE but define without".

Mark them as preserved for bincompat.

Replace their guts with calls to _STD lgamma(), which preserves the interface but improves the behavior.

StephanTLavavej avatar Nov 20 '25 23:11 StephanTLavavej

I think we can close #5577 once this is merged.

JMazurkiewicz avatar Nov 20 '25 23:11 JMazurkiewicz

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

StephanTLavavej avatar Nov 25 '25 18:11 StephanTLavavej