safetensors: accept the VMLX_ spelling of the mmap variables - #4
Open
rcfa wants to merge 1 commit into
Open
Conversation
`mmap_tensor_buffers_enabled` already reads both `VMLINUX_` and `VMLX_`, and `VMLX_QMV_MR` in the Metal backend uses the current prefix. The other five mmap variables were left on the legacy spelling alone, so which name works depends on which knob you reach for. It matters beyond consistency, because the Swift side has already moved: vmlx-swift's `ModelFactory` sets `VMLX_MMAP_SAFETENSORS` to force mmap for a load, and nothing here reads it. That write reaches nothing today; mmap keeps working only because `MLX_SAFETENSORS_MMAP` is set beside it. For this one variable the NEW name is the weaker one. All three spellings are accepted; nothing that works today stops working. The two value-returning knobs differ from the booleans and are handled accordingly: for `||`-chained flags precedence is unobservable, but `mmap_start_cold_pct` and `mmap_cold_advice` RETURN a value, so the current spelling is consulted first — otherwise setting both would silently yield the legacy one. Companion to osaurus-ai/vmlx-swift#322, which finishes the same migration on the Swift side. Once this lands, the legacy write can be dropped there.
rcfa
marked this pull request as ready for review
August 28, 2026 19:21
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
mmap_tensor_buffers_enabledhere already reads bothVMLINUX_andVMLX_,and
VMLX_QMV_MRin the Metal backend uses the current prefix. The other fivemmap variables were left on the legacy spelling alone, so which name works
depends on which knob you reach for.
All three spellings are accepted after this; nothing that works today stops
working.
Why it isn't only consistency
The Swift side has already moved.
vmlx-swift'sModelFactorydoes:to force mmap for a load — and nothing here reads that name:
So that write reaches nothing. mmap keeps working only because
MLX_SAFETENSORS_MMAPis set beside it. For this one variable the new nameis the weaker one — a user who sets
VMLX_MMAP_SAFETENSORSgets less than onewho sets the legacy spelling.
Booleans and values are not the same problem
For the
||-chained flags, precedence is unobservable, so the new spelling issimply added — the shape
mmap_tensor_buffers_enabledalready uses.mmap_start_cold_pctandmmap_cold_advicereturn a value, so order isobservable. There the current spelling is consulted first; appending it after
the legacy one would mean that setting both silently yields the old value.
Companion PR
osaurus-ai/vmlx-swift#322 finishes the same migration on the Swift side. Until
this lands, that PR writes both spellings at each
setenvsite so mmap keepsbeing forced regardless of which mlx revision is pinned. Once this merges, the
legacy write can be dropped there.
A note on the base branch
This targets
qwen4-exp, notmain— the mmap safetensors code does notexist on
main, andqwen4-expis the branchvmlx-swiftpins as itsCmlxsubmodule (currently
0c7cf413). Happy to retarget if this work is headed formainby another route.Verified by compiling through the
vmlx-swiftbuild, which is what consumesthis file.