github-starred/ollama
2
0
Fork 0
mirror of https://github.com/ollama/ollama.git synced 2026-05-22 21:51:37 -05:00
Code Issues 16.9k Packages Projects Releases 122 Wiki Activity

[PR #15745] [CLOSED] x/mlxrunner/model: add baseline test coverage for quant metadata scanning #61986

New Issue
Closed
opened 2026-04-29 16:56:51 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-04-29 16:56:51 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/ollama/ollama/pull/15745
Author: @jodagreyhame
Created: 4/22/2026
Status: ❌ Closed

Base: main ← Head: pr3/mlxrunner-model-baseline-tests

📝 Commits (2)

  • 3d9e540 x/mlxrunner: recognise mlx-lm plural aux naming at load time
  • 8a98f6e x/mlxrunner/model: add baseline test coverage for quant metadata scanning

📊 Changes

9 files changed (+723 additions, -28 deletions)

View changed files

📝 x/mlxrunner/model/embedding.go (+14 -3)
📝 x/mlxrunner/model/embedding_test.go (+32 -0)
📝 x/mlxrunner/model/linear.go (+18 -3)
➕ x/mlxrunner/model/linear_test.go (+172 -0)
➕ x/mlxrunner/model/quant_test.go (+154 -0)
📝 x/mlxrunner/model/root.go (+6 -1)
➕ x/mlxrunner/model/root_test.go (+147 -0)
📝 x/mlxrunner/runner.go (+68 -21)
➕ x/mlxrunner/runner_test.go (+112 -0)

📄 Description

Summary

The x/mlxrunner/model package ships substantial quant-metadata logic with embedding_test.go as the sole test file. This PR adds targeted tests for the remaining exported and package-private functions that exist on main today, filling a coverage gap without changing any production code.

Zero production code is touched. Every test passes against the current model/ package unmodified. The PR is self-contained and independent of the naming-recognition and config.json-override PRs.

Scope

This PR covers baseline behaviour in the model/ package that is common across all three PRs in the series — i.e. behaviour that holds regardless of whether the naming-recognition PR or the config-overrides PR has been merged:

  • QuantizationParams — the type → (group_size, bits, mode) lookup.
  • TensorQuantParams — the nil-map / missing-key / empty-QuantType fallbacks and the GroupSize override branch. Does not cover the Bits/Mode field semantics (those are introduced by the config-overrides PR and tested there).
  • ResolveLinearQuantParams — the non-affine shape-inference skip and the ambiguous 64/32 hint-bits tiebreak. Does not cover the new per-tensor-fields branch added by the config-overrides PR.
  • InferAffineQuantParamsFromShapes — all explicit and fallback branches.
  • parseGlobalQuantMetadata — present / absent / malformed metadata.
  • mainTensorNames — the Ollama-native singular-suffix filter. Plural-suffix filter extension is tested in the naming-recognition PR.
  • inferQuantTypeFromShapes — the <tensorName>.scale lookup path (singular).
  • readBlobTensorQuantInfo — both the Ollama-native happy path and the four error paths.
  • MakeLinearLayer — dense fallback, Ollama-native quantised, and the existing per-tensor {QuantType, GroupSize} override semantics.

Explicitly not in scope:

  • Plural aux naming (<module>.scales / <module>.biases) — introduced and tested by the naming-recognition PR.
  • config.json quant overrides and the new TensorQuantInfo.Bits/.Mode field semantics — introduced and tested by the config-overrides PR.
  • Any behaviour that only becomes true after one of the sibling PRs merges.

Order of merge doesn't matter: the assertions in this PR only describe behaviour that both sibling PRs preserve.

Why

The package's behaviour is nuanced enough that regression guards are worth having in place before any follow-up touches these functions. In particular:

  • Shape-based affine quant inference has several distinct branches (explicit 32/64 match, the 64/32 ambiguous pair resolved by hint bits, and the isCommonGroupSize fallback in both directions) — none tested.
  • readBlobTensorQuantInfo has four error paths (missing file, truncated header, oversize header, malformed JSON) and an Ollama-native happy path — neither tested outside of the indirect coverage via embedding_test.go.
  • QuantizationParams's internal uppercasing and the exact default-branch behaviour for unknown types are load-bearing for several other call sites and weren't locked in.

Fixing these in isolation also makes future edits in this package safer to review.

What's covered

quant_test.go (new, ~300 lines)

  • TestQuantizationParams_Types — table over known quant type strings, plus lowercase coverage confirming internal uppercasing, plus an unknown-type case.
  • TestTensorQuantParams_Fallback — nil map, missing key, empty QuantType entry.
  • TestTensorQuantParams_UsesQuantTypeWhenPresent — entry with {QuantType, GroupSize} resolves to the expected params (the GroupSize > 0 branch; does not touch the Bits/Mode fields added by the config-overrides PR).
  • TestResolveLinearQuantParams_PerTensorHit — non-affine mode skips shape inference.
  • TestResolveLinearQuantParams_AffineShapeInference — affine mode triggers inference for the ambiguous groupSize4==64 && groupSize8==32 case, resolved via hintBits.
  • TestInferAffineQuantParamsFromShapes — 8 sub-cases covering: explicit groupSize4==32 branch, explicit groupSize8==64 branch, both isCommonGroupSize directions (16 for bits=4, 128 for bits=8), the ambiguous case with hintBits of 0/4/8, both-uncommon rejection, nil weight, nil scales.

root_test.go (new, ~380 lines)

  • TestReadBlobTensorQuantInfo_OllamaNativeSingularNaming — canonical Ollama dot-child format; asserts tensorQuant populated correctly, globals populated.
  • TestMainTensorNames_SkipsSingularAuxSuffixes — .scale and .bias filtered out of the main names list. (Plural-suffix filter coverage is in the naming-recognition PR.)
  • TestInferQuantTypeFromShapes_SingularAuxName — current code path uses header[tensorName+".scale"]; asserts inferred type for a valid shape.
  • TestParseGlobalQuantMetadata — 4 sub-cases: no metadata key, empty metadata, populated metadata, non-numeric group_size.
  • TestReadBlobTensorQuantInfo_ErrorPaths — 4 sub-cases: missing file, truncated header size, oversize header (>100 MB guard), malformed JSON.

Includes a co-located unexported test helper fakeSafetensorsBlob(t, header) that writes a binary safetensors-style header to a temp path.

linear_test.go (new, ~170 lines)

  • TestMakeLinearLayer_DenseFallback_NoWeight — nil returned on empty map or bias-only map.
  • TestMakeLinearLayer_DenseLinear_NoScales — weight only → *nn.Linear.
  • TestMakeLinearLayer_DenseLinear_WithBias — weight + bias wired through.
  • TestMakeLinearLayer_OllamaNativeQuantized — singular naming (<path>.weight_scale) → *nn.QuantizedLinear with default quant params.
  • TestMakeLinearLayer_OllamaNativeQuantized_WithQBiasAndBias — both weight_qbias and bias wired through the struct.
  • TestMakeLinearLayer_PerTensorQuantOverride_CurrentBehaviour — a {QuantType:"INT4", GroupSize:32} entry in tensorQuant overrides the global (16, 4, "nvfp4") default to (32, 4, "affine").
  • TestMakeLinearLayer_NilTensorQuant_UsesDefaults — nil tensorQuant map passes defaults through.
  • TestLinearFactory_Make_ProducesLayer — delegation equivalence check between factory and direct call.

Plural-aux naming tests are intentionally absent from this PR. They belong to the naming-recognition PR, which introduces the production support they verify.

Test plan

  • go test ./x/mlxrunner/model/... on macOS arm64 with MLX — all pass.
  • go test ./x/mlxrunner/model/... on Linux / CI without MLX — MLX-dependent cases skip via existing skipIfNoMLX; pure-Go cases pass.
  • go vet ./x/mlxrunner/model/... — clean.

Files touched

x/mlxrunner/model/quant_test.go     new, ~300 lines
x/mlxrunner/model/root_test.go      new, ~380 lines
x/mlxrunner/model/linear_test.go    new, ~170 lines

No production code changes.

Risk

Zero — test-only.

Relationship to other PRs

Fully independent. Lands cleanly before, after, or between the naming-recognition PR and the config.json-overrides PR.

  • If this PR lands first: the other two PRs add new test files/cases that extend this coverage.
  • If either of the other PRs lands first: this PR stays valid — its test expectations only describe branches that both sibling PRs preserve (the new Bits/Mode field semantics and plural-aux naming are deliberately out of scope here).

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/ollama/ollama/pull/15745 **Author:** [@jodagreyhame](https://github.com/jodagreyhame) **Created:** 4/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `pr3/mlxrunner-model-baseline-tests` --- ### 📝 Commits (2) - [`3d9e540`](https://github.com/ollama/ollama/commit/3d9e54097a9cd44e8ad92897836084cc6874f3ef) x/mlxrunner: recognise mlx-lm plural aux naming at load time - [`8a98f6e`](https://github.com/ollama/ollama/commit/8a98f6e62ea4d160d6f47845a8cf6a7511147a6e) x/mlxrunner/model: add baseline test coverage for quant metadata scanning ### 📊 Changes **9 files changed** (+723 additions, -28 deletions) <details> <summary>View changed files</summary> 📝 `x/mlxrunner/model/embedding.go` (+14 -3) 📝 `x/mlxrunner/model/embedding_test.go` (+32 -0) 📝 `x/mlxrunner/model/linear.go` (+18 -3) ➕ `x/mlxrunner/model/linear_test.go` (+172 -0) ➕ `x/mlxrunner/model/quant_test.go` (+154 -0) 📝 `x/mlxrunner/model/root.go` (+6 -1) ➕ `x/mlxrunner/model/root_test.go` (+147 -0) 📝 `x/mlxrunner/runner.go` (+68 -21) ➕ `x/mlxrunner/runner_test.go` (+112 -0) </details> ### 📄 Description ## Summary The `x/mlxrunner/model` package ships substantial quant-metadata logic with `embedding_test.go` as the sole test file. This PR adds targeted tests for the remaining exported and package-private functions that exist on main today, filling a coverage gap without changing any production code. Zero production code is touched. Every test passes against the current `model/` package unmodified. The PR is self-contained and independent of the naming-recognition and config.json-override PRs. ## Scope This PR covers baseline behaviour in the `model/` package that is common across all three PRs in the series — i.e. behaviour that holds regardless of whether the naming-recognition PR or the config-overrides PR has been merged: - `QuantizationParams` — the type → `(group_size, bits, mode)` lookup. - `TensorQuantParams` — the nil-map / missing-key / empty-`QuantType` fallbacks and the `GroupSize` override branch. Does **not** cover the `Bits`/`Mode` field semantics (those are introduced by the config-overrides PR and tested there). - `ResolveLinearQuantParams` — the non-affine shape-inference skip and the ambiguous 64/32 hint-bits tiebreak. Does **not** cover the new per-tensor-fields branch added by the config-overrides PR. - `InferAffineQuantParamsFromShapes` — all explicit and fallback branches. - `parseGlobalQuantMetadata` — present / absent / malformed metadata. - `mainTensorNames` — the Ollama-native singular-suffix filter. Plural-suffix filter extension is tested in the naming-recognition PR. - `inferQuantTypeFromShapes` — the `<tensorName>.scale` lookup path (singular). - `readBlobTensorQuantInfo` — both the Ollama-native happy path and the four error paths. - `MakeLinearLayer` — dense fallback, Ollama-native quantised, and the existing per-tensor `{QuantType, GroupSize}` override semantics. Explicitly **not** in scope: - Plural aux naming (`<module>.scales` / `<module>.biases`) — introduced and tested by the naming-recognition PR. - `config.json` quant overrides and the new `TensorQuantInfo.Bits`/`.Mode` field semantics — introduced and tested by the config-overrides PR. - Any behaviour that only becomes true after one of the sibling PRs merges. Order of merge doesn't matter: the assertions in this PR only describe behaviour that both sibling PRs preserve. ## Why The package's behaviour is nuanced enough that regression guards are worth having in place before any follow-up touches these functions. In particular: - Shape-based affine quant inference has several distinct branches (explicit 32/64 match, the 64/32 ambiguous pair resolved by hint bits, and the `isCommonGroupSize` fallback in both directions) — none tested. - `readBlobTensorQuantInfo` has four error paths (missing file, truncated header, oversize header, malformed JSON) and an Ollama-native happy path — neither tested outside of the indirect coverage via `embedding_test.go`. - `QuantizationParams`'s internal uppercasing and the exact default-branch behaviour for unknown types are load-bearing for several other call sites and weren't locked in. Fixing these in isolation also makes future edits in this package safer to review. ## What's covered ### `quant_test.go` (new, ~300 lines) - `TestQuantizationParams_Types` — table over known quant type strings, plus lowercase coverage confirming internal uppercasing, plus an unknown-type case. - `TestTensorQuantParams_Fallback` — nil map, missing key, empty `QuantType` entry. - `TestTensorQuantParams_UsesQuantTypeWhenPresent` — entry with `{QuantType, GroupSize}` resolves to the expected params (the `GroupSize > 0` branch; does not touch the Bits/Mode fields added by the config-overrides PR). - `TestResolveLinearQuantParams_PerTensorHit` — non-affine mode skips shape inference. - `TestResolveLinearQuantParams_AffineShapeInference` — affine mode triggers inference for the ambiguous `groupSize4==64 && groupSize8==32` case, resolved via `hintBits`. - `TestInferAffineQuantParamsFromShapes` — 8 sub-cases covering: explicit `groupSize4==32` branch, explicit `groupSize8==64` branch, both `isCommonGroupSize` directions (16 for bits=4, 128 for bits=8), the ambiguous case with `hintBits` of 0/4/8, both-uncommon rejection, nil weight, nil scales. ### `root_test.go` (new, ~380 lines) - `TestReadBlobTensorQuantInfo_OllamaNativeSingularNaming` — canonical Ollama dot-child format; asserts `tensorQuant` populated correctly, globals populated. - `TestMainTensorNames_SkipsSingularAuxSuffixes` — `.scale` and `.bias` filtered out of the main names list. (Plural-suffix filter coverage is in the naming-recognition PR.) - `TestInferQuantTypeFromShapes_SingularAuxName` — current code path uses `header[tensorName+".scale"]`; asserts inferred type for a valid shape. - `TestParseGlobalQuantMetadata` — 4 sub-cases: no metadata key, empty metadata, populated metadata, non-numeric `group_size`. - `TestReadBlobTensorQuantInfo_ErrorPaths` — 4 sub-cases: missing file, truncated header size, oversize header (>100 MB guard), malformed JSON. Includes a co-located unexported test helper `fakeSafetensorsBlob(t, header)` that writes a binary safetensors-style header to a temp path. ### `linear_test.go` (new, ~170 lines) - `TestMakeLinearLayer_DenseFallback_NoWeight` — nil returned on empty map or bias-only map. - `TestMakeLinearLayer_DenseLinear_NoScales` — weight only → `*nn.Linear`. - `TestMakeLinearLayer_DenseLinear_WithBias` — weight + bias wired through. - `TestMakeLinearLayer_OllamaNativeQuantized` — singular naming (`<path>.weight_scale`) → `*nn.QuantizedLinear` with default quant params. - `TestMakeLinearLayer_OllamaNativeQuantized_WithQBiasAndBias` — both `weight_qbias` and `bias` wired through the struct. - `TestMakeLinearLayer_PerTensorQuantOverride_CurrentBehaviour` — a `{QuantType:"INT4", GroupSize:32}` entry in `tensorQuant` overrides the global `(16, 4, "nvfp4")` default to `(32, 4, "affine")`. - `TestMakeLinearLayer_NilTensorQuant_UsesDefaults` — nil `tensorQuant` map passes defaults through. - `TestLinearFactory_Make_ProducesLayer` — delegation equivalence check between factory and direct call. Plural-aux naming tests are intentionally absent from this PR. They belong to the naming-recognition PR, which introduces the production support they verify. ## Test plan - [ ] `go test ./x/mlxrunner/model/...` on macOS arm64 with MLX — all pass. - [ ] `go test ./x/mlxrunner/model/...` on Linux / CI without MLX — MLX-dependent cases skip via existing `skipIfNoMLX`; pure-Go cases pass. - [ ] `go vet ./x/mlxrunner/model/...` — clean. ## Files touched ``` x/mlxrunner/model/quant_test.go new, ~300 lines x/mlxrunner/model/root_test.go new, ~380 lines x/mlxrunner/model/linear_test.go new, ~170 lines ``` No production code changes. ## Risk Zero — test-only. ## Relationship to other PRs Fully independent. Lands cleanly before, after, or between the naming-recognition PR and the config.json-overrides PR. - If this PR lands first: the other two PRs add new test files/cases that extend this coverage. - If either of the other PRs lands first: this PR stays valid — its test expectations only describe branches that both sibling PRs preserve (the new `Bits`/`Mode` field semantics and plural-aux naming are deliberately out of scope here). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-29 16:56:51 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
amd
api
app
bug
build
cli
cloud
compatibility
context-length
create
docker
documentation
embeddings
feature request
feedback wanted
good first issue
gpt-oss
gpu
harmony
help wanted
image
install
intel
js
launch
linux
macos
memory
mlx
model
needs more info
networking
nvidia
ollama.com
performance
pull-request
Mirrored from GitHub Pull Request
 python
question
registry
rendering
thinking
tools
top
vulkan
windows
wsl
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/ollama#61986
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?