-
Notifications
You must be signed in to change notification settings - Fork 123
Move memory related API from GPU into Memory namespace. #296
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
base: main
Are you sure you want to change the base?
Conversation
- work for #294 - the underlying mlx API has moved from being GPU related to being Memory related - old API remains with deprecation
| /// ### See Also | ||
| /// - <doc:running-on-ios> | ||
| /// - ``Memory`` | ||
| public enum GPU { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the old GPU.swift file cut down:
- the methods and properties that are now in
Memoryare deprecated and cover the implementations inMemory - the GPU (metal) specific code remains
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this!
| mlx_set_cache_limit(¤t, cacheLimit) | ||
| /// ### See Also | ||
| /// - ``memoryLimit`` | ||
| public static var cacheLimit: Int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: previously you would read from this property and write via set(cacheLimit:) but that isn't quire idiomatic swift. Using the property for both get/set is.
| /// | ||
| /// See [the documentation](https://ml-explore.github.io/mlx/build/html/dev/metal_debugger.html) | ||
| /// for more information. | ||
| public static func startCapture(url: URL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the GPU parts.
incertum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM wrt new split and approach. @awni will be better positioned to double check the changes and approve the PR.
I'll rebase my other PR once this is merged, thanks a bunch @davidkoski!
| /// ### See Also | ||
| /// - <doc:running-on-ios> | ||
| /// - ``Memory`` | ||
| public enum GPU { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this!
@incertum