Reorganize MLXFast, MLXFFT, MLXRandom and MLXLinalg #205
Conversation
we could potentially just implement these in MLXFFT and call the ones in MLX. That gives a place to hang the deprecation and keeps old code building. |
|
This part in the docs probably needs to be updated: though we can always pick that up after the change if needed. |
| import Cmlx | ||
| import Foundation | ||
|
|
||
| public enum MLXLinalg { |
There was a problem hiding this comment.
Just to make sure we like the name, these are now:
MLX.MLXLinalg.norm()
or (without the package name):
MLXLinalg.norm()
Would this look better instead?
MLX.Linalg.norm()
Linalg.norm()
I don't have a strong opinion either way. @barronalex @awni
There was a problem hiding this comment.
Ah, but in fact we do want it the way you have it -- that means the no package version: MLXLinalg.norm() looks like the old package qualified version. I forgot :-)
There was a problem hiding this comment.
Can one use them as free functions or does it have to be prefixed with the enum type?
I must admit, having to use name MLXLinalg.norm everywhere is a bit unfortunate..
There was a problem hiding this comment.
If they are in the enum they always need the MLXLinalg.
I think we have two options there:
-
just like I suggested for FFT we leave free functions in MLXLinalg package and forward them here
-
we don't use the enum namespace here + we do the forwards from MLXLinalg
The first option let's us move to MLXLinalg.norm over time, but that assumes we want to eventually require the namespace.
The second option allows for both, though thinking about it if you import both MLX and MLXLinalg the norm function will be ambiguous and you will need to qualify it with MLXLinalg anyway. That isn't quite what we were going for. If you only import MLX it would work as expected.
| /// | ||
| /// ### See Also | ||
| /// - <doc:MLXFFT> | ||
| public func fft(_ array: MLXArray, n: Int? = nil, axis: Int = -1, stream: StreamOrDevice = .default) |
There was a problem hiding this comment.
See comment in PR top level thread -- if we implement these we can:
- put the deprecation marker on them
- call to the new implementation in MLX to not break code that uses the old name
329e443 to
d37d482
Compare
d37d482 to
2fa883c
Compare
Builds upon #204
This PR deprecates the
MLXFast,MLXFFT,MLXRandomandMLXLinalgmodules.Instead those methods are now available under the main
import MLXpackage.becomes:
or:
The old way will still work, but you will now get a deprecation message.