Two suspicious `mtl`-related behaviours about metal.jl HOT 6 CLOSED

For now I think a decent solution would be to add Int128/UInt128 to check_eltype and then make adapt_storage specific to Float64 which would have the (intended) side-effect of BigFloat erroring for not being stored inline.

from metal.jl.

mtl[] does not seem to adapt datatypes like mtl([]) does

This functionality is terrible and should be removed. mtl is a function, mtl[...] is a horrible pun. I forgot I had ported that over from CUDA.jl, where IIRC I've removed it since.

from metal.jl.

I'm the one that ported it in #194. I probably noticed it didn't behave like CUDA and took the opportunity to "fix" it. My apologies. What about the second thing? If it's intended I'll repurpose the pull request to remove mtl[...] only.

from metal.jl.

Ah, I see. Unless you feel strongly, I would vote for removing it again (even though that would be a breaking change, so we should verify nobody is relying on it). It's IMO not a good API.

mtl(Float16[]) does not promote up to Float32 but mtl(ComplexF16[]) promotes up to ComplexF32

Yeah that seems wrong, neither should promote. I guess the same bug will exist in CUDA.jl as well, and I wonder if we shouldn't just restrict the scope of the promotion to only Float64->Float32 and the complex equivalents (so that it doesn't cover Float16 and whatever we add in the future): https://github.com/JuliaGPU/CUDA.jl/blob/7a9a0a2a6becc4fba27fc664d1051194037a41e6/src/array.jl#L737-L748

from metal.jl.

Ah, I see. Unless you feel strongly, I would vote for removing it again (even though that would be a breaking change, so we should verify nobody is relying on it). It's IMO not a good API.

Not at all. I've updated the PR to remove it.

I wonder if we shouldn't just restrict the scope of the promotion to only Float64->Float32 and the complex equivalents (so that it doesn't cover Float16 and whatever we add in the future)

Float16 promotion is explicitly covered here. Would explicitly preventing promotion on the supported types and then defaulting to promoting to Float64 for any other AbstractFloat be better? That way types like BigFloat also get converted to Float32.

from metal.jl.

That way types like BigFloat also get converted to Float32.

I know it behaves like that right now, but I'm not sure that's great. Generally, we're pretty strict wrt. (dis)allowing types in GPU kernels, so transparently supporting BigFloat when working with arrays (and losing a bunch of precision because of that conversion) seems questionable to me. The exception has been Float64->Float32 because of the massive performance difference on consumer hardware.

from metal.jl.