search

JuliaGPU / Metal.jl

Metal programming in Julia
MIT License
338 stars 34 forks source link

Audit exports/public symbols #359

Open christiangnrd opened 3 weeks ago

christiangnrd commented 3 weeks ago

https://github.com/JuliaGPU/Metal.jl/pull/357#discussion_r1628311525 and writing up #358 made me realize that we might want to clean up what Metal.jl exports.

Currently, MTL is reexported. Do we want that? ~What if we didn't, and everything in the module were renamed so that they would be qualified without redundancy. Ex. MTLDevice -> MTL.Device. I don't actually know if this would be a good idea, but I would be curious to know what people think.~

MPS is not reexported. Do we want to reexport it? Regardless, which functions/objects of MPS should be considered public?

There are some inconsistencies between the way device and current_device work in Metal.jl and CUDA.jl, do we want to bring the functionality closer? (See #366)

These are just a few examples I can think of that might be worth discussing. This might not lead anywhere, but I figured I'd start a conversation.

maleadt commented 3 weeks ago

Currently, MTL is reexported. Do we want that?

Ideally not, but the package wasn't ready for that initially, i.e., you needed to use lower-level APIs to get to basic functionality. We should probably revisit that.

(A similar argument can be made for CUDA.jl)

What if we didn't, and everything in the module were renamed so that they would be qualified without redundancy. Ex. MTLDevice -> MTL.Device. I don't actually know if this would be a good idea, but I would be curious to know what people think.

I personally wouldn't do that. One of my goals was for people familiar with the Metal APIs to feel at home using Metal.jl (the same applies to CUDA/CUDA.jl). So after importing MTL it should be possible to do as much as possible using names and semantics that are very similar (but more high-level) than what Metal itself offers.

MPS is not reexported. Do we want to reexport it? Regardless, which functions/objects of MPS should be considered public?

Similar situation as MTL (so I wouldn't reexport it if needed), with the difference that even Metal.jl doesn't use anything of the MPS submodule directly I think (it's all interface methods implemented inside of the MPS module). So in principle, nothing even needs to be exported at all.

There are some inconsistencies between the way device and current_device work in Metal.jl and CUDA.jl, do we want to bring the functionality closer?

FWIW, oneAPI.jl also uses device, so I guess it makes sense to switch Metal.jl to that too.