-
Notifications
You must be signed in to change notification settings - Fork 334
Codable macro #316
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?
Codable macro #316
Conversation
Libraries/Embedders/Pooling.swift
Outdated
| import MLXNN | ||
|
|
||
| public struct PoolingConfiguration: Codable { | ||
| public struct PoolingConfiguration: Codable, Sendable { |
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.
All of these should be Sendable as well
| import ReerCodable | ||
|
|
||
| // port of https://github.com/ml-explore/mlx-examples/blob/main/llms/mlx_lm/models/cohere.py | ||
| // port of https://github.com/ml-explore/mlx-lm/tree/main/mlx_lm/models/cohere.py |
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.
Updated the urls -- these moved to the new mlx-lm repo
Libraries/MLXLLM/Models/Cohere.swift
Outdated
| @CodingKey("layer_norm_eps") public var ropeTheta: Float = 8000000.0 | ||
| @CodingKey("logit_scale") public var ropeTraditional: Bool = true | ||
| @CodingKey("rope_traditional") public var ropeScaling: [String: StringOrNumber]? = nil | ||
| @CodingKey("rope_scaling") public var logitScale: Float = 0.0625 |
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 is the typical change: use @CodingKey and the defaults just work
| swift --version | ||
| find . -name Package.resolved -exec rm {} \; | ||
| xcodebuild test -scheme mlx-libraries-Package -destination 'platform=OS X' | ||
| xcodebuild test -scheme mlx-libraries-Package -destination 'platform=OS X' -skipMacroValidation |
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 is how we tell CI to trust the macro package -- it runs in a sandbox but still requires some permission.
|
@DePasqualeOrg since you implement a lot of models what do you think about this change? It makes it much easier to add defaults in configuration at the cost of using a macro. Note: I think it requires a bit of work to be back in building shape after a recent merge up from |
|
That looks nice! This has always been difficult for me to get right. |
11bdc9f to
b76f9e4
Compare
- make all configuration Sendable and public var as well
b76f9e4 to
9a6d9a3
Compare
|
@awni I think this is ready for review when you have time. This makes it much easier to write the code to read configs and in particular have default values. |
|
这是来自QQ邮箱的假期自动回复邮件。你好,我最近正在休假中,无法亲自回复你的邮件。我将在假期结束后,尽快给你回复,或登录https://www.koudaiui.com,提交您得问题留言,或加微信gpbox7070
|
|
Saw this after the Qwen 3 VL port; this will be pretty cool to have! |
use ReerCodable macro to allow for default values