-
Notifications
You must be signed in to change notification settings - Fork 166
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
Several fixes for Random on long lists in 64-bit GAP #781
Conversation
frankluebeck
commented
May 5, 2016
- removes the kernel function RandomListMT
- adjusts RandomList to documentation
- fixes several problems by using new RandomList instead of RANDOM_LIST
- ensures that RANDOM_LIST is only called for lists of length < 2^28
- introduces a simple scheme for test code that runs differently on 32bit and 64bit GAP
- adds some test code for Random on lists
As reported by Stefan Kohl, RandomListMT did not work as expected in 64-bit GAP on lists of length >= 2^28. Kernel function removed, corresponding GAP function now uses Random(1,Length(list)).
RANDOM_LIST (compiled function from lib/random.g) does not work as expected in 64-bit GAP on list of length >= 2^28 (it only returns positions which are equal mod 2^28). RandomList did not use the Mersenne twister as is explicitly documented (but was synonym to RANDOM_LIST). - Now RandomList works as documented. - RandomList is used in several places of the library instead of RANDOM_LIST. - Functions for the GlobalRandomSource and GAPRandomSource now use RANDOM_LIST only for lists of length < 2^28.
Introduced two directories for code that behaves differently in 32-bit GAP and 64-bit GAP. Only one is used in tst/teststandard.g depending on the current GAP.
@james-d-mitchell does the partial permutations test code rely on some (previously wrong?) behaviour of |
This all looks good and all correct to me, better than my previous pull request. |
@markuspf I don't think so, but who knows. I plan to rewrite those tests when I get some time! |
I suppose we can just use the usual kludge of updating the test for the time being... |
yes, it's only two lines in |
This certainly leads to diffs in manual examples - I will update these within a couple of days:
|
This PR has been made into master but then cherry-picked onto stable-4.8. That's why it belongs to GAP 4.8.4 milestone. |