Closes #3086: Add choice to random number generators

[stress-tess]

This PR (closes #3086) adds choice to random number generators.

I had to make a very slight modification to sampleDomWeighted (just moving the increment to be after indexing) to avoid an out of bounds error.

[stress-tess]

The tests are passing for me with chpl 2.0. sample wasn't introduced until 2.0, so I'm calling choice in the compat modules. It looks like the earlier versions are compiling correctly but failing on test_choice_flags. I wonder if this is due to an issue in choice or an issue with the way I'm calling it.

[jeremiah-corrado]

@stress-tess, looking at your code nothing is standing out as incorrect. I'm guessing there is an issue with choice itself. I'll checkout your branch and see if I can figure it out.

[jeremiah-corrado]

Yep, unfortunately it looks like there is a subtle bug in choice where it can call the module-level shuffle proc (which generates a new random seed), rather than the random stream's shuffle method. This only comes up in the unweighted case when replace=false and the number of samples is greater than log2(pop_size).

patch for Chapel 1.33...

diff --git a/modules/standard/Random.chpl b/modules/standard/Random.chpl
index 51e8d8723f..b5c50ed4c2 100644
--- a/modules/standard/Random.chpl
+++ b/modules/standard/Random.chpl
@@ -948,7 +948,7 @@ module Random {
           }
         } else {
           var indices: [X] int = X;
-          shuffle(indices);
+          stream.shuffle(indices);
           for i in samples.domain {
             samples[i] = (indices[X.dim(0).orderToIndex(i)]);
           }

I'm not sure whether this is worth fixing, but I suppose the solution would be to add a correct implementation of choice to Arkouda instead of calling the implementation in the Random module.

[stress-tess]

Yep, unfortunately it looks like there is a subtle bug in choice where it can call the module-level shuffle proc (which generates a new random seed), rather than the random stream's shuffle method. This only comes up in the unweighted case when replace=false and the number of samples is greater than log2(pop_size).

patch for Chapel 1.33...
I'm not sure whether this is worth fixing, but I suppose the solution would be to add a correct implementation of choice to Arkouda.

thanks so much for running this down for me @jeremiah-corrado!! Unfortunately I think most of our users haven't made the jump to 2.0 yet, so I do think adding a correct implementation of choice is prob the way to go even tho that does not seem like the most fun task lol

I was thinking I might skip that test for now and then add the correct choice impl in a separate issue and re-enable the test then. @Bears-R-Us/arkouda-core-dev , what are y'all's thoughts on that?

Captured in #3118

[ajpotts]

Looks good to me! I like how you used the chi-squared test :)