Skip to content

fix: close loader and LoRA follow-ups from #2438 and #2439 - #2446

Merged
inureyes merged 5 commits into
mainfrom
fix/issue-2445-loader-lora-followups
Oct 19, 2026
Merged

inureyes merged 5 commits into
mainfrom
fix/issue-2445-loader-lora-followups

Conversation

@inureyes

@inureyes inureyes commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Summary

Closes #2445. Five follow-ups left by #2438 and #2439, in one PR as the issue asks, plus one blocker found while verifying item 1 on a real checkpoint.

Changes

  1. kv_arch reads Nemotron-H layer_types (src/execution/kv_arch.rs). For model_type nemotron_h without a hybrid_override_pattern, the layers_block_type / layer_types array is mapped through NEMOTRON_H_BLOCK_NAMES (now pub(crate) in src/models/nemotron_h.rs, not copied), * entries are counted, and the layer count is the array length, the way NemotronHConfig::post_init derives it. linear_attention is an M layer and is no longer counted by the contains("attention") filter.
    • Blocker found during verification: estimate_kv_arch parsed config.json with plain serde, and Nemotron-H configs such as nvidia-nemotron-3-nano-30b-a3b-4bit carry time_step_limit: [0.0, Infinity], so the KV estimate was "unavailable" for them, pattern or not (main reports KV cache: 0 bytes (unavailable ...) for that checkpoint). The file now goes through sanitize_config_json first, as the model loader does. Without this, item 1 has no effect on a real checkpoint. This also makes the estimate available for any other config that carries Infinity / NaN, which used to report 0 KV bytes.
  2. No stale-index warning on OptiQ checkpoints (src/lib/mlxcel-core/src/weights.rs, src/loading/vlm.rs). optiq_vision_sidecar_path moves into mlxcel_core::weights (same validation: relative, normal components only, .safetensors), and loading::vlm reuses it. collect_shard_paths drops exactly the declared sidecar from the index's shard list before validate_index_shards, and only when it names a subdirectory path (a top-level name is an ordinary shard). The config is read with the same Infinity / NaN replacement the model loader applies. An undeclared subdirectory entry, a malformed sidecar path or an unreadable config leaves the list unchanged, so those still fail validation and take the glob fallback as before.
  3. Text-only Muse Glimmer refuses image and video markers (src/multimodal/muse_glimmer_runtime.rs, src/models/muse_glimmer.rs, src/loading/vlm_muse_glimmer.rs, src/server/model_worker.rs, src/multimodal/vlm_runtime.rs). The check is split into a helper over token ids. MuseGlimmerTextWrapper keeps the checkpoint's image_token_id / video_token_id (two i32 fields, set by load_muse_glimmer_text), and reject_muse_glimmer_text_only_markers runs at both sites for LoadedModel::MuseGlimmer. The error says the checkpoint is loaded text-only.
  4. Integer LoRA factors no longer abort fusion (src/lora/loader.rs). compute_lora_delta casts both factors to f32 before the matmul, as runtime_lora (at claim) and src/lora/embedding.rs do. The delta was already f32 because the scale is an f32 array, so for a floating-point pair only the matmul's input precision changes; for an f32 adapter (what mlx_lm.lora writes) nothing changes.
  5. Nemotron-H surfaces the LoRA width error (src/models/nemotron_h.rs). The shared-expert up_proj / down_proj and the two latent projections return None only when {prefix}.weight is absent; otherwise the constructor error is propagated with ?, so the claim_linear width error from fix(lora): pin runtime LoRA term widths on quantized linear bases #2438 is the load error instead of a later "unclaimed" failure.

Tests

Each new test fails on the reverted fix (each fix reverted in place, test binary rebuilt, then restored):

Test Reverted result
execution::kv_arch::tests::nemotron_h_layer_types_match_the_equivalent_pattern panics: no estimate for the layer_types config
execution::kv_arch::tests::a_config_file_with_infinity_still_estimates panics: no estimate (serde rejects Infinity)
weights::tests::test_collect_shard_paths_skips_only_the_declared_optiq_sidecar (mlxcel-core) left: ["model.safetensors", "stray.safetensors"], glob fallback taken
loading::vlm::muse_glimmer::tests::text_only_muse_glimmer_refuses_image_and_video_markers panics: text-only model accepted marker (call sites reverted)
lora::loader::tests::an_integer_lora_pair_fuses_like_the_same_values_in_f32 process abort, exit 134: [matmul] Only inexact types are supported but int32 and int32 were provided
models::nemotron_h::tests::a_wrong_width_lora_term_on_an_optional_projection_fails_the_load panics: load succeeded without the shared experts

The Nemotron-H test is cfg(not(all(target_os = "macos", feature = "metal"))), since Metal refuses runtime LoRA for this model before any layer is built.

Verification

GB10, --features cuda, test-fast profile, every GPU run under gpu-lock, MLXCEL_SDPA_DETERMINISTIC=1. "Main" is a binary built from 74f9ae1 in the same worktree; "fixed" is this branch.

  • cargo test --profile test-fast --features cuda --lib -- kv_arch lora:: muse_glimmer nemotron_h loading::vlm multimodal:: model_worker memory_estimate --test-threads=1: 877 passed, 0 failed.
  • cargo test --profile test-fast -p mlxcel-core --features cuda --lib -- weights:: runtime_lora --test-threads=1: 38 passed.
  • cargo clippy --profile test-fast --features cuda --lib --tests -- -D warnings, cargo fmt --all -- --check, and the five contract checks (check_llama_compat_manifest, check_kernel_dtype_keys, check_kernel_port_dispatch, check_crate_versions, insert_apache_header --check): clean.

Real checkpoints (~/models_archive/mlx/...):

Item Checkpoint and command Main Fixed
1 nvidia-nemotron-3-nano-30b-a3b-4bit, mlxcel inspect -m KV cache: 0 bytes (unavailable ...) 48.0 MiB (hybrid: 6 attention layer(s) hold KV)
1 same checkpoint with a transformers 5 style config (hybrid_override_pattern rewritten as layer_types, num_hidden_layers removed), mlxcel inspect -m 0 bytes (unavailable ...) 48.0 MiB (hybrid: 6 attention layer(s) hold KV), identical to the pattern config
2 lfm2.5-vl-3b-optiq-4bit, mlxcel generate, text prompt and --image tests/fixtures/test_image_shapes.png stale-index warning printed no warning; generated text identical to main for both prompts
2 muse-glimmer-30b-optiq-4bit, mlxcel generate --image ... --show-reasoning stale-index warning printed no warning; generated text identical to main
3 text-only view of muse-glimmer-30b-optiq-4bit (top-level shards, index without the sidecar entries, no optiq_vision; detected as MuseGlimmer), prompt "<|image|> What is in the picture?" accepted, generates from the marker ids Error: this Muse Glimmer checkpoint is loaded text-only (no vision tower), but the prompt contains image or video marker tokens ..., exit 1
3 same view, plain prompt generates identical text
4 llama-3.2-1b-instruct-bf16 (dense), mlxcel generate --adapter, q_proj adapter of layers 14 and 15 trained with mlx_lm.lora, factors rounded to round(x * 100) and stored as int32, scale divided by 1e4 terminate called after throwing an instance of 'std::invalid_argument' ... [matmul] Only inexact types are supported, exit 134 loads and generates; text identical to the same integer values stored as f32
4 same, f32 twin adapter generates identical text to main
5 nvidia-nemotron-3-nano-30b-a3b-4bit (n_shared_experts: 1, so every MoE layer builds its shared experts through the new path), mlxcel generate --show-reasoning, 24 tokens generates identical text

The CLI's media preparation goes through prepare_request_vlm_embeddings, the server worker site, so the item 3 run exercises that call site. The unit test covers the prepare_vlm_embeddings_with_cache site and the helper the worker calls; no unit test drives prepare_request_vlm_embeddings itself, so the worker arm is covered only by the real run.

Not in this PR

Review found the same declared-sidecar entry treated as a bad shard in two other places, outside this issue's scope: src/downloader/completeness.rs (missing_indexed_shards counts optiq/optiq_vision.safetensors as missing, so a store-resolved OptiQ snapshot classifies as incomplete) and src/server/webui/catalog_fs.rs (reports it as an unsafe shard path). weight_footprint_bytes also leaves the sidecar's bytes out of the memory preflight. Both can reuse mlxcel_core::weights::optiq_vision_sidecar_path.

- kv_arch: map a Nemotron-H `layers_block_type` / `layer_types` array through the model's own block-name table (now `pub(crate)`), count `*` layers and take the layer count from the array length, so a transformers 5 config without `num_hidden_layers` gets an estimate and `linear_attention` is not counted as attention.
- weights: move `optiq_vision_sidecar_path` into `mlxcel_core::weights` and drop exactly the declared OptiQ vision sidecar from the index shard list before validation, so OptiQ checkpoints resolve through their index instead of printing the stale-index warning. `loading::vlm` reuses the helper.
- Muse Glimmer: split the marker check into a helper over token ids, keep the checkpoint's image and video ids on the text wrapper, and refuse marker tokens for the text-only load at both the worker and the VLM preparation sites.
- LoRA fusion: cast both factors to f32 before the delta matmul, as the runtime and embedding paths do, so an integer pair no longer aborts the process.
- Nemotron-H: build the four optional projections as absent only when their weight is absent and propagate a constructor error, so the runtime LoRA width error from `claim_linear` reaches the user.
…2445)

Nemotron-H configs carry `time_step_limit: [0.0, Infinity]`, which serde rejects, so `estimate_kv_arch` reported the KV cache as unavailable for every such checkpoint and the `layer_types` support had no effect on a real one. Apply `sanitize_config_json`, as the model loader does.
Clippy `manual_contains` on the split marker helper.
@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions priority:low Low priority area:models Model architectures, weights, loading, metadata status:review Under review labels Oct 18, 2026
#2445)

Review follow-ups: read `config.json` with the loader's `Infinity` / `NaN` replacement so a Nemotron-H-based OptiQ config still declares its sidecar, and drop the declared sidecar only when it names a subdirectory path, since a top-level name is an ordinary shard the index path loads.
@inureyes inureyes added status:done Completed and removed status:review Under review labels Oct 18, 2026
@inureyes
inureyes merged commit e1c50fa into main Oct 19, 2026
27 checks passed
@inureyes
inureyes deleted the fix/issue-2445-loader-lora-followups branch October 19, 2026 01:00
inureyes added a commit that referenced this pull request Oct 19, 2026
OptiQ conversions index their vision tensors to the sidecar `config.json` declares under `optiq/`. #2446 taught the loader to accept that entry, but three other checks still treated it as a missing or unsafe shard: the downloader classified a store copy as incomplete (an `--offline` run failed and a normal run re-downloaded), the WebUI catalog reported "safetensors index contains an unsafe shard path", and `weight_footprint_bytes` fell back to `model.safetensors` and left out the sidecar.

`mlxcel_core::weights::declared_optiq_vision_sidecar` is now the one config read all four sites share. The downloader and the catalog check the declared sidecar at its relative path and report it missing by name when absent; undeclared subdirectory and traversal entries are still rejected. The catalog fingerprint now covers the sidecar. The footprint drops the entry before index validation and adds the sidecar header, since `metadata.total_size` omits it.

On lfm2.5-vl-3b-optiq-4bit and muse-glimmer-30b-optiq-4bit, inspect reports 2811046368 and 23847540736 bytes (main: 1958475776 and 20003849216), offline store resolution succeeds, and the catalog marks both complete. The other 191 catalog entries are unchanged.

Closes #2451
inureyes added a commit that referenced this pull request Oct 19, 2026
…oader (#2455)

OptiQ conversions index their vision tensors to the sidecar that `config.json` `optiq_vision.sidecar` declares (`optiq/optiq_vision.safetensors`). #2446 made the loader accept that entry; three other checks still treated it as a missing or unsafe shard. On main a store copy of an OptiQ checkpoint classifies `Incomplete` (an `--offline` run fails, a normal run re-downloads), the WebUI catalog marks it incomplete with "safetensors index contains an unsafe shard path", and `mlxcel inspect` leaves the vision tower out of the weight footprint.

- `mlxcel_core::weights::declared_optiq_vision_sidecar(dir)` is split out of #2446's private helper: the Infinity/NaN-tolerant config read plus `optiq_vision_sidecar_path` validation, subdirectory paths only. The loader's `collect_shard_paths` and the three sites below all call it. The read is capped at 16 MiB because the catalog calls it on directories it does not control.
- Downloader `missing_indexed_shards`: the declared sidecar counts as present when `dir.join(sidecar)` is non-empty and is reported missing under its relative name otherwise. Undeclared subdirectory and traversal entries are still missing. Config is read only when the index names a multi-component entry.
- WebUI catalog: `indexed_completeness` and `push_index_shard_facts` skip the unsafe-path refusal for the declared sidecar only, so completeness is decided by the shared rule above and the content fingerprint now covers the sidecar file.
- `weight_footprint_bytes` drops the sidecar before index validation and adds its header bytes when the file exists. With a sidecar declared it sums the main shard headers instead of trusting `metadata.total_size`, which OptiQ writes without the sidecar (and a converter that counted it would otherwise be counted twice). Non-OptiQ checkpoints take exactly the old path.

## Verification (GB10, checkpoints under `~/models_archive/mlx`)

| Check | main (fixes disabled) | this PR |
|---|---|---|
| `mlxcel inspect -m lfm2.5-vl-3b-optiq-4bit` | 1958475776 bytes | 2811046368 bytes |
| `mlxcel inspect -m muse-glimmer-30b-optiq-4bit` | 20003849216 bytes | 23847540736 bytes |
| `mlxcel-server --offline -m mlx-community/<name> --model-store-root <store>` (store entry for each OptiQ checkpoint) | "no cached snapshot" error | resolves from the store (LFM serves `/health`; Muse reaches model loading) |
| `GET /ui-api/v1/catalog` with `--models-dir ~/models_archive/mlx --webui` | both OptiQ entries `complete: false`, unsafe shard path | both `complete: true`, no reason |

The expected totals equal main header plus sidecar header, computed independently from the safetensors headers (sidecars 852570592 and 3843691520 bytes). Non-OptiQ checks are unchanged: the other 191 catalog entries have identical completeness and fingerprints, inspect footprints for gemma-3-4b-it-4bit, glm-4.5v-4bit (stale index), llama-3.2-1b-instruct-bf16 and qwen3.5-0.8b-optiq-4bit (no sidecar) match main, and gemma-3-4b-it-4bit still resolves offline from the store.

Five new tests fail with their fix disabled and pass with it: completeness present/absent, catalog OptiQ layout, catalog fingerprint, footprint (including a `total_size` that already counts the sidecar, and a stale index), and the bounded config read. The two undeclared/traversal-path tests are guards and pass both ways. `downloader::`, `webui::catalog`, `execution::memory_estimate`, mlxcel-core `weights::`, clippy (`--lib --tests -D warnings`, main crate and mlxcel-core), fmt and the contract scripts pass locally.

Closes #2451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:models Model architectures, weights, loading, metadata priority:low Low priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: Close loader and LoRA follow-ups from #2438 and #2439

1 participant