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

Feature/add mlx #73

Merged
merged 14 commits into from
Oct 29, 2024
Merged

Feature/add mlx #73

merged 14 commits into from
Oct 29, 2024

Conversation

LeonNissen
Copy link
Contributor

@LeonNissen LeonNissen commented Oct 16, 2024

Replace Llama.cpp with MLX

♻️ Current situation & Problem

Llama.cpp has been a reliable, multiplatform option. However, Apple’s recent release of MLX provides significant performance improvements, especially optimized for Apple Silicon devices. This PR proposes migrating to MLX to leverage these optimizations.

⚙️ Release Notes

Note: MLX-Swift is currently incompatible with simulators, so testing and deployment require a physical Apple Silicon device. Additionally, when using MLX with large language models (LLMs), the Increase Memory Limit Entitlement must be added to the application’s entitlements for optimal functionality.

This update removes certain APIs from the existing configuration. Please ensure your implementation aligns with the new MLX API.

🚀 Benchmark

Benchmarks indicate substantial performance gains across all tested devices. Refer to the figure below for detailed results:

output

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LeonNissen Thank you for the nice additions and working on this PR; very important and great to move to MLX for this package!

I have added some higher-level comments that I identified and added to the git diff.

Please ensure that all UI tests are passing + REUSE and SwiftLint elements are fixed. I also observe a lot of Swift 6 warning in the PR. I know that @paulhdk is also having an open PR where he is addressing most of them; please ensure that you sync with him and @philippzagar to ensure that these elements are in sync.

Apart from this I am happy to see this merged and addressed in this PR; we might want to tag a 2.0 beta version after this is merged and we had some time to bake it a bit together with the PRs from @paulhdk 🚀

Package.swift Show resolved Hide resolved
Sources/SpeziLLMLocal/Helpers/LLMModel+numParameters.swift Outdated Show resolved Hide resolved
Sources/SpeziLLMLocal/LLMLocalPlatform.swift Show resolved Hide resolved
Sources/SpeziLLMLocal/LLMLocalSession+Setup.swift Outdated Show resolved Hide resolved
Sources/SpeziLLMLocal/Resources/Localizable.xcstrings Outdated Show resolved Hide resolved
Sources/SpeziLLMLocalDownload/LLMLocalLoadingManager.swift Outdated Show resolved Hide resolved
@PSchmiedmayer PSchmiedmayer added the enhancement New feature or request label Oct 16, 2024
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the updates; let me know once you need an additional round of reviews and have identified a way to pass all UI tests and remaining checks 🚀

Sources/SpeziLLMLocal/LLMLocalPlatform.swift Show resolved Hide resolved
@LeonNissen LeonNissen changed the base branch from integration/mlx to main October 29, 2024 02:01
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @LeonNissen; amazing additions and thank you for incorporating all the feedback!

Sources/SpeziLLMLocal/LLMLocalPlatform.swift Show resolved Hide resolved
Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 0% with 269 lines in your changes missing coverage. Please review.

Project coverage is 36.24%. Comparing base (6633d8a) to head (a251d8a).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...urces/SpeziLLMLocal/LLMLocalSession+Generate.swift 0.00% 93 Missing ⚠️
...peziLLMLocalDownload/LLMLocalDownloadManager.swift 0.00% 46 Missing ⚠️
...es/SpeziLLMLocal/Configuration/LLMLocalModel.swift 0.00% 38 Missing ⚠️
Sources/SpeziLLMLocal/LLMLocalSession+Setup.swift 0.00% 34 Missing ⚠️
Sources/SpeziLLMLocal/LLMLocalSession.swift 0.00% 19 Missing ⚠️
...SpeziLLMLocal/Helpers/LLMModel+numParameters.swift 0.00% 16 Missing ⚠️
Sources/SpeziLLMLocal/LLMLocalPlatform.swift 0.00% 8 Missing ⚠️
.../Configuration/LLMLocalPlatformConfiguration.swift 0.00% 5 Missing ⚠️
...s/SpeziLLMLocalDownload/LLMLocalDownloadView.swift 0.00% 5 Missing ⚠️
...eziLLMLocal/Configuration/LLMLocalParameters.swift 0.00% 2 Missing ⚠️
... and 2 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   30.88%   36.24%   +5.36%     
==========================================
  Files          67       64       -3     
  Lines        3002     2503     -499     
==========================================
- Hits          927      907      -20     
+ Misses       2075     1596     -479     
Files with missing lines Coverage Δ
...ocal/Configuration/LLMLocalContextParameters.swift 0.00% <ø> (ø)
Sources/SpeziLLMLocal/LLMLocalError.swift 0.00% <ø> (ø)
...cal/Configuration/LLMLocalSamplingParameters.swift 0.00% <0.00%> (ø)
...eziLLMLocal/Configuration/LLMLocalParameters.swift 0.00% <0.00%> (ø)
Sources/SpeziLLMLocal/LLMLocalSchema.swift 0.00% <0.00%> (ø)
.../Configuration/LLMLocalPlatformConfiguration.swift 0.00% <0.00%> (-100.00%) ⬇️
...s/SpeziLLMLocalDownload/LLMLocalDownloadView.swift 0.00% <0.00%> (ø)
Sources/SpeziLLMLocal/LLMLocalPlatform.swift 0.00% <0.00%> (-40.62%) ⬇️
...SpeziLLMLocal/Helpers/LLMModel+numParameters.swift 0.00% <0.00%> (ø)
Sources/SpeziLLMLocal/LLMLocalSession.swift 0.00% <0.00%> (ø)
... and 4 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6633d8a...a251d8a. Read the comment docs.

@PSchmiedmayer PSchmiedmayer merged commit a4ee2da into main Oct 29, 2024
17 of 18 checks passed
@PSchmiedmayer PSchmiedmayer deleted the feature/add-MLX branch October 29, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants