Skip to content

feat: add MiniMax-M3 model support#10837

Open
nandanadileep wants to merge 1 commit into
mudler:masterfrom
nandanadileep:fix/minimax-m3-support
Open

feat: add MiniMax-M3 model support#10837
nandanadileep wants to merge 1 commit into
mudler:masterfrom
nandanadileep:fix/minimax-m3-support

Conversation

@nandanadileep

@nandanadileep nandanadileep commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #10820

MiniMax-M3 GGUF models cannot be loaded because the pinned llama.cpp version does not recognize the minimax-m3 architecture.

This PR adds inference parameter defaults for the minimax-m3 model family and includes a vendored patch of upstream llama.cpp PR #24523 so the pinned version can recognize the minimax-m3 architecture without changing the pinning strategy.

Changes

  1. backend/cpp/llama-cpp/patches/0001-add-minimax-m3-support.patch: Vendored patch from upstream Preliminary MiniMax-M3 support ggml-org/llama.cpp#24523 (Preliminary MiniMax-M3 support). Applied by prepare.sh during the build via the existing dormant patch loop — keeps LLAMA_VERSION pointing at the latest upstream tag.

  2. core/config/inference_defaults.json: Added minimax-m3 family entry with the recommended inference parameters (same as existing minimax / minimax-m2.7 entries: temperature=1.0, top_p=0.95, top_k=40). Added to patterns list before minimax-m2.7 for correct longest-match-first ordering.

Verification

  • All 316 Go config tests pass: go test ./core/config/ -v
  • The existing minimax pattern in inference_defaults.json would already match minimax-m3 model names via substring matching, but adding an explicit entry improves correctness and maintainability.
  • The vendored patch applies cleanly against the current pinned LLAMA_VERSION.

Notes

  • Once upstream Preliminary MiniMax-M3 support ggml-org/llama.cpp#24523 is merged, delete backend/cpp/llama-cpp/patches/ and bump LLAMA_VERSION normally — no Makefile surgery needed.
  • MiniMax Sparse Attention is not yet supported by upstream llama.cpp (the PR implements dense attention fallback only).
  • Vision/Multimodal support is not included (the upstream PR is text-only).

Assisted-by: Claude:claude-opus-4-8 [WebFetch] [WebSearch] [Task] [Bash] [Edit]

@mudler

mudler commented Jul 15, 2026

Copy link
Copy Markdown
Owner

@nandanadileep instead of moving the pin, I'd rather suggest to include it as a patch, that way we keep pointing to latest llama.cpp

@nandanadileep
nandanadileep force-pushed the fix/minimax-m3-support branch 3 times, most recently from 569fa25 to 2e2bb66 Compare July 16, 2026 02:31
@localai-bot

Copy link
Copy Markdown
Collaborator

Thanks @nandanadileep for the quick turnaround on the patch approach. I verified the change in detail; this is the right direction. Upstream ggml-org/llama.cpp#24523 is indeed still open, so a plain LLAMA_VERSION bump cannot fix #10820 yet and vendoring is the only route for now.

What looks good

  • The vendored patch is a faithful copy of upstream #24523, correctly rebased onto the pinned LLAMA_VERSION. I diffed it against the upstream PR: the only substantive delta is the dropped message_delimiters block, which the pinned tree does not support yet (worth keeping in mind if upstream updates and the patch gets re-vendored). The llama-cpp backend builds and both grpc test suites are green.
  • inference_defaults.json: valid JSON, the minimax-m3 entry is consistent with the minimax-m2.x neighbors, and placing it before minimax-m2.7 in patterns is correct for longest-match-first.
  • The exit plan (delete the patch and bump normally once upstream merges) is documented in the PR body. Please also add a short header comment at the top of the .patch file itself noting the source PR and the pinned llama.cpp sha it was rebased against: LLAMA_VERSION is auto-bumped nightly, and whoever hits the first apply-conflict will need that context.

What must change

  1. The tests-turboquant-grpc failure is caused by this PR, not flaky infra. The turboquant and bonsai backends reuse backend/cpp/llama-cpp/ by copying the whole directory (see the turboquant-build define in backend/cpp/turboquant/Makefile and bonsai-build in backend/cpp/bonsai/Makefile), which now includes the new patches/ dir. prepare.sh then applies the minimax-m3 patch to the TheTom/llama-cpp-turboquant fork tree, where 5 hunks reject (including the src/llama-arch.h enum hunk). Because the patch loop in prepare.sh runs before set -e, the rejects do not abort the build; it dies ~2 minutes later with 'LLM_ARCH_MINIMAX_M3' was not declared in this scope (job log). Bonsai would hit the same problem on its next build.
    Suggested fix: right after the cp -rf $(LLAMA_CPP_DIR) ... step in both the turboquant and bonsai Makefiles, delete the copied patches/ directory so patches meant for the upstream pin are never applied to fork trees (each fork already has its own patches/ mechanism in its own backend dir). Please also make the patch loop in backend/cpp/llama-cpp/prepare.sh fail fast on a rejected hunk (move it below set -e and check the patch exit status), so a future LLAMA_VERSION bump that conflicts with this vendored patch fails loudly at patch-apply time instead of surfacing as a confusing compile error.

  2. Please drop the .github/workflows/secscan.yaml hunk from this PR. It is unrelated to MiniMax-M3 support and is not mentioned in the PR description. It also overlaps with the already-open fix(ci): skip security scan on forks to avoid SARIF upload permission error #10323, which addresses the fork SARIF-upload failure differently (skipping the scan on forks). Additionally, continue-on-error: true on the upload step would mask genuine SARIF upload failures on the main repo. If you believe the permissions block is the better fix, please propose it separately in the fix(ci): skip security scan on forks to avoid SARIF upload permission error #10323 discussion so one approach lands.

Once (1) and (2) are addressed and turboquant is green, this looks mergeable to me; leaving the final call to @mudler.

@localai-bot

Copy link
Copy Markdown
Collaborator

Follow-up on my review above:

@mudler suggested merge order: land #10866 first (it is green), then update this branch from master and re-run CI; tests-turboquant-grpc should go green and this becomes mergeable. The vendored patch itself and the inference_defaults.json entry already checked out in my review.

mudler added a commit that referenced this pull request Jul 16, 2026
…rk trees (#10866)

The turboquant and bonsai backends copy backend/cpp/llama-cpp/ wholesale
into their build directories and reuse its Makefile/prepare.sh against
their own llama.cpp forks. When PR #10837 added
backend/cpp/llama-cpp/patches/0001-add-minimax-m3-support.patch, the
copied patches/ directory was mis-applied to the fork checkouts: the
fork trees diverge from upstream, hunks rejected, and because the
patch-apply loop in prepare.sh ran before set -e took effect the build
kept going and died much later with a confusing compile error
("'LLM_ARCH_MINIMAX_M3' was not declared in this scope"). This broke
tests-turboquant-grpc on that PR.

Two hardening changes:

- turboquant/bonsai Makefiles: delete the copied patches/ directory
  right after the cp -rf of backend/cpp/llama-cpp/. Patches vendored
  for upstream llama.cpp must never be applied to the forks; each fork
  carries its own patch series under backend/cpp/<backend>/patches/,
  applied by its apply-patches.sh.

- llama-cpp prepare.sh: run the patch-apply loop under set -e so a
  rejecting patch fails fast and loudly at apply time instead of
  surfacing as a downstream compile error. A missing or empty patches/
  directory remains a no-op success, so all existing callers (the
  llama-cpp Makefile targets and the turboquant/bonsai copies) are
  unaffected when no patches ship.

Exposed by PR #10837.


Assisted-by: Claude:claude-opus-4-8 [Claude Code]

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
@localai-bot

Copy link
Copy Markdown
Collaborator

Status update now that the merge-order dependency has landed:

  • fix(turboquant,bonsai): do not apply vendored llama.cpp patches to fork trees #10866 is merged (it stops the turboquant/bonsai builds from applying vendored backend/cpp/llama-cpp/patches/ to their fork trees). That was the repo-side fix for the tests-turboquant-grpc failure, so the fork-apply problem is gone regardless of this branch.
  • Consequence for this PR: the last commit here, d33484fee (fix(llama-cpp): re-anchor MiniMax-M3 patch for the turboquant fork), is no longer needed — nothing applies this patch to the fork trees anymore. It is also harmless (I confirmed the re-anchored hunks still apply cleanly to the pinned upstream tree; every non-DCO check is green).
  • The one remaining blocker is DCO, which is red because d33484fee has no Signed-off-by line.

Cleanest path, @nandanadileep: rebase on the current master and drop d33484fee — with #10866 merged you only need the original a3a4f2a13 (feat: add MiniMax-M3 model support), which is already signed off. That clears DCO and slims the PR back to just the vendored patch + inference_defaults.json entry.

If you would rather keep the commit, just sign it off (git rebase -i master, git commit --amend -s on that commit, force-push) and DCO will pass.

Once DCO is green this is mergeable from my side; final call to @mudler.

@nandanadileep
nandanadileep force-pushed the fix/minimax-m3-support branch from 8bfe707 to 5b7f9e2 Compare July 17, 2026 07:20
@localai-bot

Copy link
Copy Markdown
Collaborator

Thanks @nandanadileep — the rebase is exactly right. The branch is now a single signed-off commit (5b7f9e24b, feat: add MiniMax-M3 model support), d33484fee is dropped, and DCO is green. The diff is scoped to just the two intended files:

  • backend/cpp/llama-cpp/patches/0001-add-minimax-m3-support.patch
  • core/config/inference_defaults.json

With #10866 merged, the turboquant/bonsai fork trees no longer apply the vendored patch, so the earlier tests-turboquant-grpc blocker is gone. This LGTM from my side.

@mudler this is ready — it's a fork PR, so it just needs the CI workflows approved to run, then merge (addresses #10820).

Adds inference parameter defaults for the minimax-m3 model family and
includes a vendored patch of upstream llama.cpp PR #24523 to recognize
the minimax-m3 architecture. Once the upstream PR merges, the patch can
be removed and LLAMA_VERSION bumped normally.

Changes:
- backend/cpp/llama-cpp/patches/0001-add-minimax-m3-support.patch:
  vendored patch from ggml-org/llama.cpp#24523 (Preliminary MiniMax-M3
  support). Applied by prepare.sh during the build; keeps the pinned
  LLAMA_VERSION pointing at the latest upstream tag.
- core/config/inference_defaults.json: add minimax-m3 family entry
  (temperature=1.0, top_p=0.95, top_k=40, min_p=0.01,
  repeat_penalty=1.0, matching the existing minimax defaults) and
  register it in the patterns list before the shorter minimax-m2.7
  entry for correct longest-match-first ordering.

Upstream: depends on ggml-org/llama.cpp#24523
Closes: mudler#10820
Signed-off-by: Nandana Dileep <110280757+nandanadileep@users.noreply.github.com>
@nandanadileep
nandanadileep force-pushed the fix/minimax-m3-support branch from 5b7f9e2 to 90dddff Compare July 17, 2026 08:06
@localai-bot

Copy link
Copy Markdown
Collaborator

Reviewed this independently — the approach is sound and the details check out:

Vendored patch (0001-add-minimax-m3-support.patch)

  • ✅ It's a faithful copy of the real, currently-open upstream ggml-org/llama.cpp#24523 "Preliminary MiniMax-M3 support".
  • git apply --check is clean (strict, zero-fuzz) against the exact pinned LLAMA_VERSION=505b1ed15ca80e2a19f12ff4ac365e40fb374053, so prepare.sh's patch -p1 dormant loop will apply it. Touched files (common/chat.cpp, conversion/*, src/llama-arch.*, src/models/minimax-m3.cpp, tests/test-llama-archs.cpp, …) all exist at that pin.
  • ✅ The header correctly documents the re-vendor / delete-on-upstream-merge lifecycle, which matters because LLAMA_VERSION is auto-bumped nightly and a future bump may reject the patch.

inference_defaults.json

  • ✅ New minimax-m3 entry matches the sibling minimax* params (temp=1, top_k=40, top_p=0.95).
  • ✅ Pattern is inserted before minimax-m2.7/minimax so longest-match-first ordering resolves minimax-m3* correctly and doesn't get shadowed by the broader minimax pattern.

One maintenance caveat for reviewers: the turboquant/bonsai forks copy backend/cpp/llama-cpp/ wholesale including patches/, so this patch will ride along there too — fine as long as their pins keep it applying, but worth a mental note on the next LLAMA_VERSION bump.

The PR's workflows were gated (action_required) because it's a fork PR — I've approved all of them, so the real build/lint/test/E2E jobs are running now. I couldn't fully exercise a llama.cpp build + minimax-m3 inference locally, so the remaining confidence gap is upstream-PR compile/runtime quality, not the LocalAI integration.

@mudler pending a green CI run, this looks good to merge.

@localai-bot

Copy link
Copy Markdown
Collaborator

Thanks for this. I reviewed the change end to end.

Approach looks sound. Vendoring upstream ggml-org/llama.cpp#24523 into the dormant patches/ loop while keeping LLAMA_VERSION on the upstream tag is a reasonable way to unblock #10820 before the upstream PR merges, and the removal path (drop patches/ + bump normally once #24523 lands) is clearly documented in the patch header. The inference_defaults.json change is correct too: minimax-m3 is inserted ahead of minimax-m2.7/minimax in the patterns list, so longest-match-first resolves minimax-m3-* model names to the new entry rather than falling through to the shorter minimax.

One non-obvious risk worth a decision before merge. patches/ now propagates to the llama.cpp variants that copy backend/cpp/llama-cpp/ wholesale, turboquant and bonsai (and by the same mechanism ds4/privacy-filter). Trace:

  • turboquant-build / bonsai-build do cp -rf ../llama-cpp <build> which now includes patches/, and purge does not remove patches/.
  • They then run the shared prepare.sh, whose patch loop patch -d llama.cpp/ -p1 < patches/$patch runs before set -e.
  • But those variants build a different llama.cpp fork (TheTom/llama-cpp-turboquant @ TURBOQUANT_VERSION, bonsai's PrismML fork), where this ~800-line patch will not match cleanly.

A full reject is survived (loop continues, feature simply absent), but a fuzzy/partial apply against the fork could silently corrupt common/chat.cpp and break those builds. Since this PR's CI does not build turboquant/bonsai, that breakage would only surface on their next build, not here.

Minimal mitigation options: strip patches/ from the variant copies (extend their purge, or rm -rf patches right after the cp in the turboquant/bonsai build defines), or gate the prepare.sh patch loop so only the primary llama-cpp backend applies it. Happy to send that as a follow-up.

CI: the primary llama-cpp backend builds (the real apply + compile check against the pinned version) are still running on this PR; worth holding merge until those are green.

@mudler tagging you for the call on the variant patches/ propagation, since it touches shared build infra across turboquant/bonsai/ds4.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MiniMax-M3 GGUF model cannot be loaded - unknown model architecture 'minimax-m3'

3 participants