-
Notifications
You must be signed in to change notification settings - Fork 103
Decouple from tokenizer and downloader packages #118
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
Open
DePasqualeOrg
wants to merge
34
commits into
ml-explore:main
Choose a base branch
from
DePasqualeOrg:swift-tokenizers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
6b8f237
Add embedding model loading benchmark
DePasqualeOrg b34acde
Add revision to embedders Identifier
DePasqualeOrg f343bb9
Cleanup: Remove preparePrompt
DePasqualeOrg acb0b04
Cleanup: Fix Hugging Face spelling in docs and comments
DePasqualeOrg 4a60aac
Replace swift-transformers with swift-tokenizers and swift-huggingface
DePasqualeOrg 05edb20
Implement Downloader protocol for provider agnosticism
DePasqualeOrg 272d128
Temporarily pin to optimizations branch of DePasqualeOrg/swift-huggin…
DePasqualeOrg 448adaf
Cleanup: Use empty string for default prompt
DePasqualeOrg 4e204fc
Include optional revision parameter for custom tokenizer IDs
DePasqualeOrg 435041c
Update readmes with new usage
DePasqualeOrg ecf3212
Patch swift-huggingface to improve cache hit performance
DePasqualeOrg 098b9db
Re-export HuggingFace in MLXLMHuggingFace and MLXEmbeddersHuggingFace
DePasqualeOrg ebadfd7
Implement Tokenizer protocol, factor out tokenization and downloading
DePasqualeOrg 711fd9e
Consistently use `tokenIds` for `[Int]` (align with Python transformers)
DePasqualeOrg 18bebc5
Clean up doc comments
DePasqualeOrg 56e0433
Update documentation and skills
DePasqualeOrg 1daceed
Use integration packages in tests (review before merging)
DePasqualeOrg 94aace8
Fix compiler warnings in tests
DePasqualeOrg 678b2f4
Decouple tokenizer and downloader from integration tests and benchmarks
DePasqualeOrg 6c8b0b0
Include jinja files in download glob patterns
DePasqualeOrg 380d4d4
Add download cache hit benchmarks
DePasqualeOrg c06366c
Add tool invocation tests from upstream PR #114 (0840626)
DePasqualeOrg 849c62d
Fixes for Swift 6.0 compatibility in CI
DePasqualeOrg d89eaba
Add modelDirectory and tokenizerDirectory on ModelContainer
DePasqualeOrg 64f72cf
Improve tokenizer benchmarks
DePasqualeOrg ea87cd0
Switch back to Swift Tools 5.12
DePasqualeOrg ada52c9
Centralize file download patterns
DePasqualeOrg f01bb2b
Remove unneeded imports added in #144
DePasqualeOrg f799b92
Merge branch 'main' into swift-tokenizers
DePasqualeOrg 9d95ab4
Merge main into swift-tokenizers
DePasqualeOrg 5c917c6
Merge branch 'main' into swift-tokenizers
DePasqualeOrg 63fd72d
Merge upstream/main into swift-tokenizers
DePasqualeOrg 9f1b334
add macros to help people maintain parity with current mlx-swift-lm h…
davidkoski 2fecb3a
Use release versions of integration packages in usage examples
DePasqualeOrg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
614 changes: 614 additions & 0 deletions
614
Libraries/IntegrationTestHelpers/IntegrationTestHelpers.swift
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| # Integration Test Helpers | ||
|
|
||
| `IntegrationTestHelpers` and `BenchmarkHelpers` provide shared test logic for verifying end-to-end model loading, inference, tokenizer performance, and download performance. They are designed to be used by integration packages that supply their own `Downloader` and `TokenizerLoader` implementations. | ||
|
|
||
| ## Integration packages | ||
|
|
||
| - [Swift Tokenizers MLX](https://github.com/DePasqualeOrg/swift-tokenizers-mlx): Uses [Swift Tokenizers](https://github.com/DePasqualeOrg/swift-tokenizers) and [Swift HF API](https://github.com/DePasqualeOrg/swift-hf-api) | ||
| - [Swift Transformers MLX](https://github.com/DePasqualeOrg/swift-transformers-mlx): Uses [Swift Transformers](https://github.com/huggingface/swift-transformers) and [Swift Hugging Face](https://github.com/huggingface/swift-huggingface) | ||
|
|
||
| Integration tests and benchmarks are run from those packages. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This code looks nearly identical to
ModelAdapterFactory.load(). I wonder if these can/should share implementation?It looks like the MLXEmbedders retains the top level
load()function while the LLM/VLM side migrate this intoModelFactory. Comparing againstmainit looks like that is the pattern there too, but maybe we should take this opportunity to consolidate some of this.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 investigated this and it seems it's not so straightforward, since MLXEmbedders has its own
ModelConfigurationtype.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.
Yeah, I guess it does -- the ModelConfiguration in MLXLMCommon has properties for detokenizing, so that kind of makes sense. Perhaps a task for after this PR. With a major version bump we could take a look at various APIs and see if any could be simplified or cleaned up.