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

Tracking issue for migrating to use mlx-c #35

Open
38 of 48 tasks
minghuaw opened this issue Apr 18, 2024 · 0 comments
Open
38 of 48 tasks

Tracking issue for migrating to use mlx-c #35

minghuaw opened this issue Apr 18, 2024 · 0 comments

Comments

@minghuaw
Copy link
Collaborator

minghuaw commented Apr 18, 2024

Initial implementation steps

Problems found

  • Document that mlx is lazy
  • Perform type conversion for item()
  • Perform type conversion for as_slice (Unlike item(), we cannot use mlx_astype on the inner pointer here because it creates a new mlx_array which should be freed at the end of this fn, making it the returned slice pointing at a temporary value.)
  • Split as_slice into three versions
    • as_slice<T>(&self) -> &[T] - checks if the inner dtype is the same as the output dtype, panics if different. Also panics if pointer is null
    • as_slice_unchecked<T>(&self) -> &[T] - unsafe and does not perform any check
    • try_as_slice<T>(&self) -> Result<&[T], Error> - checks the type and returns an error if the types don't match or ptr is null
  • Indexing array (read). mlx allows out of bound indexing (Possible mx.array bug?  ml-explore/mlx#206)
  • Indexing array (write). Check if out of bound indexing is still allowed
  • Revisit all error types (when everything is more or less ready) not needed with the new error handling
  • Revisit whether *_unchecked functions should be unsafe not needed with the new error handling
  • Revisit eval() (https://ml-explore.github.io/mlx/build/html/usage/lazy_evaluation.html)
    • Calling eval() on mlx_array or mlx_vector_array multiple times is allowed (is &mut really required)
    • Check if implicit eval() takes place on the c++ side
  • (Related to eval()) Whether Array should be Send and/or Sync (impl Send for Array & fix broken links in docs #64)
  • Revisit Clone for Array
  • Implement PartialEq for Array using array_eq
  • Add unit test to cover this case with indexing ( feat(api): indexing + shapes #52 (comment))
  • Avoid direct use of the Add or Sub operators (ie. +, +=, -, -=), use alternatives like saturating_add etc instead. This is probably not needed anymore with the new error handling
    • Which should be used then? saturating_*, checked_*, overflowing_*, unchecked_*, wrapping_*
  • The ops macros could probably be generated automatically. However, declarative macros cannot be defined in impl or trait blocks.
  • Review args that take ownership of Array to check if they really need ownership (if all they need is a ref, change them to .as_ref() in the corresponding macros)
  • Review default dtype (with python) for factory ops macros
  • arange is missing from the constructors for Array (feat(api): add arange #72)
  • Fix bug with as_slice when the underlying array is not contiguous #77
  • gpu support for fft ops should be available in mlx 0.15
  • What functions should take impl ScalarOrArray as argument? (see ScalarOrArray in the swift binidng)
  • Some exceptions cannot be displayed properly with the new error handling (Fixed problem with exception not displayed properly #86)
  • Revisit whether the deep clone in index and index_mut is necessary
  • Add From<complex64> for Array
@dcvz dcvz pinned this issue Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant