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

Audit exports/public symbols #359

Closed
christiangnrd opened this issue Jun 5, 2024 · 1 comment
Closed

Audit exports/public symbols #359

christiangnrd opened this issue Jun 5, 2024 · 1 comment
Labels
breaking Issues or PRs that would require a breaking release when resolved. libraries Things about libraries and how we use them. speculative Not sure if we want this.

Comments

@christiangnrd
Copy link
Contributor

christiangnrd commented Jun 5, 2024

#357 (comment) 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.

@christiangnrd christiangnrd added speculative Not sure if we want this. libraries Things about libraries and how we use them. breaking Issues or PRs that would require a breaking release when resolved. labels Jun 5, 2024
@maleadt
Copy link
Member

maleadt commented Jun 5, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Issues or PRs that would require a breaking release when resolved. libraries Things about libraries and how we use them. speculative Not sure if we want this.
Projects
None yet
Development

No branches or pull requests

2 participants