consistent treatment of HasDType (Float.self) vs dtype (.float32)#222
consistent treatment of HasDType (Float.self) vs dtype (.float32)#222davidkoski merged 3 commits intomainfrom
Conversation
davidkoski
commented
Apr 10, 2025
- fixes Missing functionality around types/dtypes #199
- there were inconsistent methods for creating MLXArrays with various patterns using Float.self vs .float32
- add an FInfo struct also in support of dtype
- encountered when porting VLMs
| /// The difference between 1.0 and the next smallest representable float larger than 1.0 | ||
| /// | ||
| /// In Swift this is e.g. ``Double.ulpOfOne`` | ||
| public var eps: Double { |
There was a problem hiding this comment.
per suggestion from Awni this exposes everything as Double. we ran into some cases in porting Qwen2.5VL where it would have been convenient to map from DType to these values.
I wonder if we actually want to return an MLXArray here?
There was a problem hiding this comment.
I think it's probably better to return it as a Double. Makes it easier to use these in functions which only take scalars as input (like linspace for example). I guess the inconvenience is that you have to specify the type when using them in creation operations like full, but that seems kind of minor.
| var result = mlx_array_new() | ||
| mlx_zeros(&result, shape.map { Int32($0) }, shape.count, T.dtype.cmlxDtype, stream.ctx) | ||
| return MLXArray(result) | ||
| MLX.zeros(shape, type: type, stream: stream) |
There was a problem hiding this comment.
This is MLXArray.zeros() -- a natural way to write things in swift. We also have the free function zeros() which does the same thing but is a little less idiomatic swift. Anyway we want:
- only one implementation -- just have one call the other
- they should take type (Float.self) or dtype (.float32) consistently -- this was not the case
Source/MLX/Factory.swift
Outdated
| return MLXArray(result) | ||
| } | ||
|
|
||
| /// Create a square identity matrix and a given ``DType``. |
There was a problem hiding this comment.
I'm not entirely sure what you are trying to say here with the Dtype, but it's confusing. If it's about the type of the output I would change to:
/// Create a square identity matrix with a given ``DType``.
Or if it's about the argument I would say:
/// Create a square identity matrix given a ``DType``.
There was a problem hiding this comment.
I think the former. Thanks for the thorough read of these doc comments!
awni
left a comment
There was a problem hiding this comment.
Left a few cosmetic comments. Otherwise good to go, thanks for the update!
- fixes #199 - there were inconsistent methods for creating MLXArrays with various patterns using Float.self vs .float32 - add an FInfo struct also in support of dtype - encountered when porting VLMs
Co-authored-by: Awni Hannun <awni@apple.com>