-
Notifications
You must be signed in to change notification settings - Fork 131
Support building MLX-Swift on Linux through SwiftPM (CPU only) #304
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?
Support building MLX-Swift on Linux through SwiftPM (CPU only) #304
Conversation
|
Amazing @Joannis! Let me check in with @davidkoski and ask when we might be ready to bump mlx-c in order to unblock current PRs. |
878fc10 to
e8dd62b
Compare
|
@Joannis you may rebase this PR now since the other PR got merged yesterday. Happy to help with the review once it's rebased. |
Signed-off-by: Melissa Kilby <mkilby@apple.com> # Conflicts: # CMakeLists.txt # README.md # Source/Examples/Example1.swift # Source/Examples/Tutorial.swift
e8dd62b to
76494af
Compare
|
Rebased @incertum |
| } | ||
|
|
||
| open func callAsFunction(_ x: MLXArray) -> MLXArray { | ||
| #if canImport(MLXFast) |
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.
@davidkoski do you have more context around MLXFast issues? Intuitively it sounds like something you would like to have. It seems to just compile fine over CMake on Linux.
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.
I am not sure what this is for. MLXFast used to be a separate package, but was folded into MLX, see Sources/MLX/MLXFast.swift.
MLXFast remains as a stub to avoid breaking things and this function is:
@available(*, deprecated, message: "layerNorm is now available in the main MLX module")
@_disfavoredOverload
public func layerNorm(
_ x: MLXArray, weight: MLXArray? = nil, bias: MLXArray? = nil, eps: Float,
stream: StreamOrDevice = .default
) -> MLXArray {
return MLXFast.layerNorm(x, weight: weight, bias: bias, eps: eps, stream: stream)
}What issue is this working around?
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.
In this case MLXFast is an enum to give a namespace to look like the old module MLXFast. So we have:
MLXFast.layerNorm(the module MLXFast if you were to import that)MLX.MLXFast.layerNormakaMLXFast.layerNorm(the new home, you should not import MLXFast and it is not here)MLX.layerNorm(just forwards toMLX.MLXFast.layerNormfor convenience)
OK, so the code here is referencing the second one as there is no import of MLXFast, thus we are getting the MLXFast scoped to the MLX module.
| @@ -0,0 +1,24 @@ | |||
| FROM swift:6.2.1-jammy AS base | |||
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.
Could we move the Dockerfile somewhere under .github for CI use or for documentation purposes?
How much work would it be to add the swift build to https://github.com/ml-explore/mlx-swift/blob/main/.github/scripts/setup%2Bbuild-linux-container-cmake.sh
or better yet a new script / CI job (my preference)?
| dependencies: [ | ||
| "MLX", | ||
| "MLXRandom", | ||
| .target(name: "MLXFast", condition: .when(platforms: [.macOS, .iOS, .tvOS, .visionOS, .watchOS])) |
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.
I think these references to MLXFast are:
- stale
- maybe causing the problems in Normalization.swift. I propose the following:
diff --git a/Package.swift b/Package.swift
index d88d9e4..0d1ed11 100644
--- a/Package.swift
+++ b/Package.swift
@@ -188,7 +188,7 @@ let package = Package(
),
.target(
name: "MLXNN",
- dependencies: ["MLX", "MLXRandom", "MLXFast"],
+ dependencies: ["MLX"],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency")
]
@@ -218,7 +218,7 @@ let package = Package(
.testTarget(
name: "MLXTests",
dependencies: [
- "MLX", "MLXRandom", "MLXNN", "MLXOptimizers", "MLXFFT", "MLXLinalg", "MLXFast",
+ "MLX", "MLXNN", "MLXOptimizers",
]
),
|
FYI #319 will have more Package.swift changes -- if we can get this merged first you won't have to figure out how to merge those in. Outstanding issues. These have to be fixed before the merge:
Ideally this would be fixed but we can do a followup PR:
|
Based on #293
Proposed changes
Checklist
Put an
xin the boxes that apply.pre-commit run --all-filesto format my code / installed pre-commit prior to committing changes