Skip to content

Avoid extra philox calls with ElementRNG - #723

Merged
maleadt merged 20 commits into
JuliaGPU:mainfrom
nhz2:nz/rng-perf-improvements
Sep 4, 2026
Merged

Avoid extra philox calls with ElementRNG#723
maleadt merged 20 commits into
JuliaGPU:mainfrom
nhz2:nz/rng-perf-improvements

Conversation

@nhz2

@nhz2 nhz2 commented May 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #721

This PR avoids some extra philox calls when using ElementRNG. ElementRNG is used as an AbstractRNG when sampling any non batched type including types with custom samplers defined in different packages.

This PR also makes ComplexF16 a batched type.

Comment thread test/testsuite/random.jl Outdated
Comment thread src/host/random.jl Outdated
Comment thread src/host/random.jl Outdated
@nhz2
nhz2 marked this pull request as draft July 24, 2026 19:37
@nhz2
nhz2 marked this pull request as ready for review July 25, 2026 05:19
@nhz2

nhz2 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

There are many interesting bugs with using Float16 and Int128, but I don't think they are related to this PR.

@nhz2

nhz2 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Due to JuliaLang/julia#62507 I'm restricting Float16 tests to Julia versions 1.13 or later.

@nhz2
nhz2 requested a review from maleadt July 27, 2026 13:13
@maleadt

maleadt commented Jul 28, 2026

Copy link
Copy Markdown
Member

Due to JuliaLang/julia#62507 I'm restricting Float16 tests to Julia versions 1.13 or later.

That's excessive. Float16 had been working fine before; why does it only fail here? If it's specific to a test, you can always add a specific version-gated filter on top of the eltypes passed into the testsuite

@nhz2

nhz2 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

The Julia 1.12 miscompile only happens on CPUs with avx512_fp16. Github actions will sometimes use a CPU with avx512_fp16 now. If you run the existing tests enough times you can see the failure. I've limited the removal of Float16 tests to only Julia 1.12 and avx512_fp16 CPUs.

@maleadt
maleadt force-pushed the nz/rng-perf-improvements branch from ef81f59 to f94690c Compare September 3, 2026 20:17
@maleadt

maleadt commented Sep 3, 2026

Copy link
Copy Markdown
Member

The Float16 issue should be solved on 1.12.7, so I removed the workaround. Also rebased the PR.

@nhz2

nhz2 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. The AMDGPU i128 problem will hopefully be fixed by either llvm/llvm-project#213523 or llvm/llvm-project#214518

@maleadt

maleadt commented Sep 4, 2026

Copy link
Copy Markdown
Member

Thanks. The AMDGPU i128 problem will hopefully be fixed by either llvm/llvm-project#213523 or llvm/llvm-project#214518

In the interim, let's do JuliaGPU/AMDGPU.jl#1066

@maleadt
maleadt merged commit 49ac504 into JuliaGPU:main Sep 4, 2026
18 checks passed
@nhz2
nhz2 deleted the nz/rng-perf-improvements branch September 4, 2026 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rand! with RNG and Complex{Int128} isn't random

2 participants