fix(models): accept an Inkling sub-config that carries a field and its alias - #1561
Merged
Conversation
…s alias inference-optimization/Inkling-0.6B-A0.6B could not be loaded at all. Config parse failed with `duplicate field 'text_hidden_size'`, and once that key was handled, with `duplicate field 'num_channels'`. InklingVisionConfig and InklingAudioConfig declare serde aliases (decoder_dmodel for text_hidden_size, n_channels for num_channels). Serde treats a field and its alias as the same field, so an object carrying both spellings is a duplicate-field error and the whole config fails to deserialize even when the two values agree. Neither spelling can be dropped, because published checkpoints disagree on which they use. Read off the three downloaded configs: Inkling-Small-mlx-4bit and Inkling-Small-NVFP4 ship only the aliases, so those are load-bearing; Inkling-0.6B-A0.6B ships both spellings of every pair at matching values and was rejected outright. Equal values now collapse to the canonical spelling. Disagreeing values are an error rather than a silent pick: the two keys name one quantity, and choosing either would surface much later as a shape mismatch. The reconciliation is driven by a list of (canonical, alias) pairs rather than a special case per key, because fixing these one at a time is exactly how num_channels went unnoticed until text_hidden_size was fixed. Both parse paths get it: InklingConfig::from_json_with_sidecar for audio_config, and loading/vlm_inkling.rs which deserializes vision_config directly off the raw value. Verified on all three real checkpoints, which now parse and load. They then abort during generation on a separate sliding-window-attention shape mismatch that this change does not address and does not introduce.
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.
Summary
inference-optimization/Inkling-0.6B-A0.6Bcould not be loaded at all. Config parse failed withduplicate field 'text_hidden_size', and once that key was handled, withduplicate field 'num_channels'.Cause
InklingVisionConfig(src/vision/encoders/inkling_hmlp.rs) andInklingAudioConfig(src/audio/inkling_tower.rs) declare serde aliases:Serde treats a field and its alias as the same field, so an object carrying both spellings is a duplicate-field error and the whole config fails to deserialize, even with the two values in agreement.
Neither spelling can be dropped
Read off the three downloaded checkpoints:
decoder_dmodeltext_hidden_sizen_channelsnum_channelsmlx-community/Inkling-Small-mlx-4bitthinkingmachines/Inkling-Small-NVFP4inference-optimization/Inkling-0.6B-A0.6BThe Inkling-Small checkpoints ship only the aliases, so the aliases are load-bearing. The 0.6B ships both spellings of every pair at matching values and was rejected outright.
Fix
Equal values collapse to the canonical spelling. Disagreeing values are an error rather than a silent pick: the two keys name one quantity, and choosing either would surface much later as a shape mismatch.
The reconciliation is driven by a list of
(canonical, alias)pairs rather than a special case per key, because fixing these one at a time is exactly hownum_channelswent unnoticed untiltext_hidden_sizewas fixed. That reasoning is recorded on the constant and asserted in the test, so the next alias added to those structs has an obvious place to go.Both parse paths get it:
InklingConfig::from_json_with_sidecarforaudio_config, andsrc/loading/vlm_inkling.rs, which deserializesvision_configdirectly off the raw config value. Fixing only the first leaves the vision half failing.Test plan
inkling_config_accepts_both_text_width_spellings: every declared pair doubled at matching values collapses to the canonical name (the real 0.6B shape); alias-only passes through untouched (the Inkling-Small shape); canonical-only passes through; contradictory values are rejected for both pairs with an error naming the section; an absent, null, or non-object sub-config is not an error.cargo test --lib models::inkling26,vision::encoders::inkling5,audio::inkling11, all passing.cargo clippy --lib --tests -- -D warningsexit 0;cargo fmt --checkclean.Verified on all three real checkpoints, which is what this bug required and what CI could not do:
NVFP4 matters as its own row: it reaches deserialization through
promote_nvfp4_config, a different path, and also passes.What this does not fix
All three checkpoints still abort during generation with
[broadcast_shapes] Shapes (1,32) and (1,8) cannot be broadcast((1,8)and(1,4)on the 0.6B). Those operands matchswa_num_attention_headsandswa_num_key_value_headsexactly on every checkpoint, so it is a sliding-window-attention GQA problem, independent of config parsing and not introduced here. Being able to reach it is the progress this PR makes. Tracked separately.Note on
mlxcel inspectinspectreported FITS for the 0.6B while it could not load, because it reads the safetensors header and never deserializes the full config. An inspect pass is not evidence that a checkpoint loads.Closes #1560
Refs #1549