- 
                Notifications
    You must be signed in to change notification settings 
- Fork 87
Fix indexing for gpu rand #641
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
base: master
Are you sure you want to change the base?
Conversation
befeb7a    to
    1d1ebd9      
    Compare
  
    | Thanks for following up! Annoyingly though, test failures seem related. | 
1d1ebd9    to
    7f818e9      
    Compare
  
    | Your PR requires formatting changes to meet the project's style guidelines. Click here to view the suggested changes.diff --git a/test/testsuite/random.jl b/test/testsuite/random.jl
index 6ecd235..bb493da 100644
--- a/test/testsuite/random.jl
+++ b/test/testsuite/random.jl
@@ -24,7 +24,7 @@
             rand!(rng, A)
             Random.seed!(rng, 1)
             rand!(rng, B)
-            @test Array(A) == Array(B) broken=SEEDING_BROKEN && (prod(d) > length(rng.state))
+            @test Array(A) == Array(B) broken = SEEDING_BROKEN && (prod(d) > length(rng.state))
 
             if rng != cpu_rng
                 rand!(cpu_rng, A)
@@ -64,7 +64,7 @@
             randn!(rng, A)
             Random.seed!(rng, 1)
             randn!(rng, B)
-            @test Array(A) == Array(B) broken=SEEDING_BROKEN && (prod(d) > (2 * length(rng.state)))
+            @test Array(A) == Array(B) broken = SEEDING_BROKEN && (prod(d) > (2 * length(rng.state)))
 
             if rng != cpu_rng
                 randn!(cpu_rng, A) | 
| The fix resurfaced #530, which wasn't detected before because we weren't testing on arrays with length greater than the maximum workgroup size. I think this should be merged once I find a way to properly mark the broken tests because that'll bring us back to where we were before the switch to KA introduced the bug. | 
5ce03b7    to
    62797c8      
    Compare
  
    | Failing Metal tests are related but I'll push a fix to mark the test broken once this is merged | 
Finish #615. The issue was that JLArrays wasn't limiting the number of workgroup threads so it was trying to access rand state past 256.
Closes #614
Closes #615