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

Parameterize MtlArray storage mode #194

Merged
merged 4 commits into from
Jun 5, 2023

Conversation

christiangnrd
Copy link
Contributor

@christiangnrd christiangnrd commented May 29, 2023

My initial attempt at #190.

Only touches MtlArray because I could not figure out how to parameterize Obj-C objects. If it's possible, let me know and I'll parameterize MTLBuffer.

Will add docs once the code is reviewed.

To change:

  • Add type parameters in MtlArray constructors in array.jl
  • Modify zeros, ones, fill in array.jl to use new interface
  • Modify rand, randn in random.jl to use new interface
  • MtlArrayAdaptor
  • Documentation

@christiangnrd christiangnrd changed the title Param mtlarray Parameterize MtlArray storage mode May 29, 2023
@christiangnrd christiangnrd changed the title Parameterize MtlArray storage mode Parameterize MtlArray storage mode May 29, 2023
src/array.jl Outdated Show resolved Hide resolved
src/pool.jl Outdated Show resolved Hide resolved
@maleadt
Copy link
Member

maleadt commented May 31, 2023

Looks good, thanks!

Only touches MtlArray because I could not figure out how to parameterize Obj-C objects. If it's possible, let me know and I'll parameterize MTLBuffer.

Which ObjC type did you want to parameterize?

@maleadt maleadt added enhancement New feature or request arrays Things about the array abstraction. labels May 31, 2023
@christiangnrd
Copy link
Contributor Author

I have addressed your review comments. I'll be adding documentation over the weekend.

Which ObjC type did you want to parameterize?

I was thinking MTLbuffer. Would there be any benefits to doing so?

@maleadt
Copy link
Member

maleadt commented Jun 1, 2023

I was thinking MTLbuffer. Would there be any benefits to doing so?

I don't think so, at least not currently.

@christiangnrd
Copy link
Contributor Author

I added documentation similar to what CUDA.jl has except that I mention the storage parameter in mtl and MtlArray.

@maleadt maleadt marked this pull request as ready for review June 5, 2023 08:34
Copy link
Member

@maleadt maleadt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Anything else you want to do here, since you hadn't marked the PR ready for review?

test/array.jl Outdated Show resolved Hide resolved
test/array.jl Outdated Show resolved Hide resolved
Fix fill method
Fix typo
@christiangnrd
Copy link
Contributor Author

I forgot to mark it ready for review. Should be good to merge if you deem it so!

@maleadt maleadt merged commit e18f0e2 into JuliaGPU:main Jun 5, 2023
@christiangnrd christiangnrd deleted the param_mtlarray branch June 5, 2023 15:29
maleadt referenced this pull request Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays Things about the array abstraction. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants