prevent random distributions from returning NaN/Inf#1228
prevent random distributions from returning NaN/Inf#1228StephanTLavavej merged 12 commits intomicrosoft:masterfrom
Conversation
MattStephanson
left a comment
There was a problem hiding this comment.
-
For
fisher_f_distribution, should we also reject here if_Px == _Ty{1}? That would generate infinity rather than NaN.
Lines 4016 to 4019 in ba6f895
-
The issue mentions
student_t_distribution, but there's no change to it. It's not obvious to me how it would cause NaN, but_Rs == 0would give infinity.
Lines 4145 to 4156 in ba6f895
|
Here is my test program with a slight improvement to the @statementreply test code on the issue description. #include <cassert>
#include <cmath>
#include <random>
#include <iostream>
#include "\tests\std\tests\GH_001017_discrete_distribution_out_of_range\bad_random_engine.hpp"
using namespace std;
template <class Distribution>
void test(Distribution&& distribution) {
for (bad_random_generator rng; !rng.has_cycled_through();) {
const auto x = distribution(rng);
assert(!isnan(x) && !isinf(x));
}
}
int main() {
test(normal_distribution<>{});
std::cout << "normal_distribution\t\tok" << std::endl;
test(lognormal_distribution<>{});
std::cout << "lognormal_distribution\t\tok" << std::endl;
test(fisher_f_distribution<>{});
std::cout << "fisher_f_distribution\t\tok" << std::endl;
test(student_t_distribution<>{});
std::cout << "student_t_distribution\t\tok" << std::endl;
std::cout << "Done!";
std::cin.get();
return 0;
} |
c28a9bb to
2d3dbe5
Compare
2d3dbe5 to
a0657ed
Compare
aad06c0 to
e370ec1
Compare
e370ec1 to
01ad653
Compare
MattStephanson
left a comment
There was a problem hiding this comment.
Nice work, this looks mostly good to me. I just have a few minor points.
Co-authored-by: MattStephanson <68978048+MattStephanson@users.noreply.github.com>
Co-authored-by: MattStephanson <68978048+MattStephanson@users.noreply.github.com>
cbezault
left a comment
There was a problem hiding this comment.
If there's something that I'm missing with regards to those static_casts then this LGTM. Otherwise could you just choose a consistent style for getting the type of variables you want?
Co-authored-by: Curtis J Bezault <curtbezault@gmail.com>
StephanTLavavej
left a comment
There was a problem hiding this comment.
This looks good to me, thanks. I have three superficial comments about comments (heh) and asserts, and one slightly more significant question about using a double versus _Ty constant. If the answer there is that comparing against 1.0e-4 is intentional and correct, I'm happy.
|
Thanks again for fixing this silent bad codegen in |
Fixes #1174