Skip to content
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

Test for min / max broadcasting issue #389

Closed
rkierulf opened this issue Jul 16, 2024 · 5 comments · Fixed by JuliaGPU/GPUCompiler.jl#602 or #391
Closed

Test for min / max broadcasting issue #389

rkierulf opened this issue Jul 16, 2024 · 5 comments · Fixed by JuliaGPU/GPUCompiler.jl#602 or #391
Assignees

Comments

@rkierulf
Copy link

rkierulf commented Jul 16, 2024

I've run into an issue broadcasting min and max together over a Metal array:

using Metal
A = Float32[1,2,3,4,5]
B = convert(MtlArray,A)
t_start, t_end = Float32(1.0), Float32(4.0)
min.(max.((A .- t_start) ./ (t_end - t_start), zero(Float32)), one(Float32))
min.(max.((B .- t_start) ./ (t_end - t_start), zero(Float32)), one(Float32))

The last two lines produce different results, with the results for the CPU array A being correct and the GPU array B being incorrect. For the GPU result, it seems to produce the same results as the line below, interestingly:

max.((B .- t_start) ./ (t_end - t_start), one(Float32))

I'm seeing this on Julia 1.10.4 with a Mac M1, and I've confirmed this issue to be the cause of this failing build: https://buildkite.com/julialang/komamri-dot-jl/builds/824 which passes for Julia 1.9, so it's possible this is a regression in Julia 1.10, but I haven't confirmed this to be the case. For our use case, we should be able to work around the issue by using a tmp array to separate the min / max broadcasts, or writing an actual kernel to do the equivalent of the failing operation.

Full version info:

Julia Version 1.10.4
Commit 48d4fd48430 (2024-06-04 10:41 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (arm64-apple-darwin22.4.0)
  CPU: 8 × Apple M1
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, apple-m1)
Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores)
@tgymnich
Copy link
Member

tgymnich commented Jul 17, 2024

Seems like the custom maximum / minimum implementation for metal in juliagpu/GPUCompiler is broken.

@christiangnrd
Copy link
Member

Bisected to JuliaLang/julia#47814.

@maleadt
Copy link
Member

maleadt commented Jul 18, 2024

Would probably be valuable to add this as a test though.

@maleadt maleadt reopened this Jul 18, 2024
@maleadt maleadt changed the title Min / max broadcasting issue Test for min / max broadcasting issue Jul 18, 2024
@maleadt maleadt removed the bug label Jul 18, 2024
@cncastillo
Copy link

I just noticed that this was fixed in GPUCompiler.jl. Would pushing a PR bumping its version be helpful?

@christiangnrd
Copy link
Member

@cncastillo The minimum supported Julia version was increased to 1.10 on master/main for both repos. I assume the next GPUCompiler version will be breaking and that's why it hasn't been bumped yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants