Skip to content

Prebuilt: carry TML Inkling on top of MiniMax-M3 for the nightly PR set - #40

Closed
danielhanchen wants to merge 5 commits into
prebuilt-base-minimax-m3from
inkling-on-minimax
Closed

Prebuilt: carry TML Inkling on top of MiniMax-M3 for the nightly PR set#40
danielhanchen wants to merge 5 commits into
prebuilt-base-minimax-m3from
inkling-on-minimax

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Companion to #39. Resolves the last conflict blocking the nightly full release build.

Why this is needed

ggml-org#24523 (MiniMax-M3) and ggml-org#25731 (TML Inkling) cannot both be merged into the prebuild tree independently. Both append a new llm_arch value immediately before LLM_ARCH_UNKNOWN, and both add a chat parser plus a template-detection block in the same regions of common/chat.cpp.

Whichever entry is applied second conflicts, and reordering does not help:

b10133 + 24423 + 24523 + 25731  ->  OK, OK, CONFLICT
b10133 + 24423 + 25731 + 24523  ->  OK, OK, CONFLICT

Conflicting files both ways: common/chat.cpp, src/llama-arch.h.

What this branch is

ggml-org/llama.cpp#24523 head (a58a7fa6) with ggml-org/llama.cpp#25731 head (453c4381) merged into it and the conflict resolved. Both sides are purely additive in common/chat.cpp against the merge base (b10107), so the resolution keeps everything from both sides:

  • src/llama-arch.h: both LLM_ARCH_MINIMAX_M3 and LLM_ARCH_INKLING
  • common/chat.cpp: both common_chat_params_init_minimax_m3 and common_chat_params_init_inkling, and both detection blocks

Additional fix

Inkling had the same stale chat API usage that MiniMax-M3 did. Upstream 910196f renamed common_chat_params::thinking_end_tag to thinking_end_tags and made it a vector, so this no longer compiled:

data.thinking_end_tag   = END_MESSAGE;   // before
data.thinking_end_tags  = {END_MESSAGE}; // after

That means ggml-org#25731 as it stands does not build against b10133 or newer either, independently of the conflict.

Verification

Built llama, llama-common, test-llama-archs and test-chat against the merged tree with zero errors.

  • test-chat: all tests passed
  • test-llama-archs: exit 0, minimax-m3 OK at 0.00e+00 (inkling reports SKIP on this backend, as several other archs do)

Merge sequence on b10133 with this branch pinned as the third entry is clean.

Lifetime

Temporary. Once either upstream PR lands, the other can be rebased and pinned directly again, and this branch should be dropped from scripts/unsloth/pr-set.json. Per the pr-set doc, keep this PR open rather than merging it into fork master, since the nightly tree is the upstream tag plus pins.

Hybrid attention model: 55 sliding-window plus 11 global layers, banded
content-dependent relative position bias instead of RoPE, per-layer short
convolution state, fine-grained MoE (256 experts top-6 plus 2 shared),
attention log-scaling past 128K, 1M context.

Includes the GGML_OP_FLASH_ATTN_EXT_BANDED operator (CPU and CUDA, fused
into the MMA flash attention kernel with an fp16 accumulator overflow
guard), HF to GGUF conversion, chat template with typed content block
parsing (interleaved thinking, narration and tool calls), mmproj vision
and audio support, and backend op tests at production shapes.
# Conflicts:
#	ggml/src/ggml-cuda/mmq.cuh
#	src/llama-model-saver.cpp
#	src/llama-vocab.h
danielhanchen pushed a commit that referenced this pull request Jul 26, 2026
Refreshing the ggml-org#25731 pin was not enough on its own. It and ggml-org#24523
conflict with each other in common/chat.cpp and src/llama-arch.h: both
append a new llm_arch value immediately before LLM_ARCH_UNKNOWN and both
add a chat parser plus a detection block in the same regions. Whichever
entry is applied second conflicts, and reordering does not help.

#40 carries ggml-org#24523 with ggml-org#25731 merged into it and the
conflict resolved, keeping every function and dispatch entry from both
sides. It also adapts Inkling to the renamed thinking_end_tags API, so
it builds against b10133 and newer.

Replaying the resolver sequence on b10133 with these pins:

  OK  ggml-org ggml-org#24423    @ c3fb972
  OK  ggml-org ggml-org#24523    @ a58a7fa
  OK  unslothai #40      @ fee66f6

Drop the #40 entry and repin ggml-org#25731 upstream once either PR lands.
@danielhanchen
danielhanchen changed the base branch from master to prebuilt-base-minimax-m3 July 26, 2026 11:35
Upstream 910196f renamed common_chat_params::thinking_end_tag to
thinking_end_tags and changed it from std::string to a vector, so
data.thinking_end_tag = END_MESSAGE no longer compiles.
…or the nightly prebuild

Both PRs add a chat parser and a template-detection block in the same
region of common/chat.cpp, immediately before "namespace workaround {",
so they cannot be merged into the prebuild tree independently:
whichever is applied second conflicts, regardless of ordering.

This branch carries the resolution. Both sides are purely additive in
common/chat.cpp against the merge base, so it keeps both
common_chat_params_init_minimax_m3 and common_chat_params_init_inkling
along with both detection blocks.

src/llama-arch.h no longer needs resolving: ggml-org#24523 now places
LLM_ARCH_MINIMAX_M3 next to LLM_ARCH_MINIMAX_M2 rather than at the tail
of the enum, so it no longer collides with LLM_ARCH_INKLING.

The thinking_end_tags adaptation is no longer carried here either; it
has landed on the Inkling branch itself.
@danielhanchen
danielhanchen force-pushed the prebuilt-base-minimax-m3 branch from a58a7fa to baee0f5 Compare July 26, 2026 12:06
danielhanchen pushed a commit that referenced this pull request Jul 26, 2026
ggml-org ggml-org#24523 now places LLM_ARCH_MINIMAX_M3 next to LLM_ARCH_MINIMAX_M2
instead of at the tail of the enum, matching how llama-arch.cpp already
groups the name table. That removes the src/llama-arch.h half of the
collision with ggml-org#25731, which appends LLM_ARCH_INKLING at the tail.

ggml-org ggml-org#25731 has picked up current master and the thinking_end_tags
rename, so it now builds on its own against b10133 and newer.

Both head SHAs moved, so the pins and the carrier branch are refreshed.
unslothai #40 now only resolves what is left: both PRs still add a chat
parser and a detection block in the same region of common/chat.cpp,
immediately before "namespace workaround {". There is no principled
alternative anchor there, since chat.cpp has no MiniMax-M2 parser to sit
beside, so that half stays with the carrier.

Replaying the resolver sequence on b10133:

  OK  ggml-org ggml-org#24423    @ c3fb972
  OK  ggml-org ggml-org#24523    @ baee0f5
  OK  unslothai #40      @ 233cedb
danielhanchen added a commit that referenced this pull request Jul 26, 2026
#39 landed the first pin refresh only, so master still points at commits
that have since moved on and that conflict with each other.

ggml-org ggml-org#24523 was force-pushed: it now places LLM_ARCH_MINIMAX_M3 next
to LLM_ARCH_MINIMAX_M2 rather than at the tail of the enum, matching how
llama-arch.cpp already groups the name table. That removes the
src/llama-arch.h half of its collision with ggml-org#25731, which appends
LLM_ARCH_INKLING at the tail. New head baee0f5.

ggml-org ggml-org#25731 has picked up current master and the thinking_end_tags
rename, so it now builds on its own against b10133 and newer. Its entry
is replaced by unslothai #40, which carries what is still unresolvable
between the two: both add a chat parser and a detection block in the
same region of common/chat.cpp, immediately before "namespace
workaround {". chat.cpp has no MiniMax-M2 parser to sit beside, so
there is no principled alternative anchor for that half.

Replaying the resolver sequence on b10133:

  OK  ggml-org ggml-org#24423    @ c3fb972
  OK  ggml-org ggml-org#24523    @ baee0f5
  OK  unslothai #40      @ 233cedb

Drop the #40 entry and repin ggml-org#25731 upstream once either PR lands.

Co-authored-by: Daniel Han <unslothai@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 233cedbe9c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread conversion/inkling.py
model_arch = gguf.MODEL_ARCH.INKLING
undo_permute = False

_SKIP_PREFIXES = ("model.visual.", "model.audio.", "model.mtp.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip top-level multimodal towers during text conversion

For checkpoints whose tower tensors use the top-level visual.* or audio.* layout, the text converter does not filter those tensors even though InklingMmprojModel.filter_tensors explicitly supports the same layout. They consequently reach the fallback map_tensor_name() call and fail with Can not map tensor, preventing creation of the language GGUF; include the bare tower prefixes in this skip list.

Useful? React with 👍 / 👎.

Comment thread common/chat.cpp
// turns may interleave narration, thinking and calls; parse block-by-block (tool block
// first) since a whole-body choice would let the text branch swallow tool blocks into
// visible content
auto mixed_body = p.one_or_more(p.choice({ tool_block, reasoning_block, text_block }));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor disabled parallel calls in mixed Inkling output

When tools use the default/auto tool choice and parallel_tool_calls is false, this unconditional one_or_more still permits multiple tool_blocks, so both the grammar and parser can accept and return several calls despite the request disabling parallel calls. Apply the same single-call constraint used by tool_calls to the mixed-content branch.

Useful? React with 👍 / 👎.

Comment thread ggml/include/ggml.h
GGML_OP_FILL,

GGML_OP_FLASH_ATTN_EXT,
GGML_OP_FLASH_ATTN_EXT_BANDED,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve RPC op IDs when adding banded attention

When an RPC client and server mix protocol patch versions 4.0.3 and 4.0.4, inserting this value in the middle of ggml_op renumbers every following operation even though negotiate_hello() accepts the connection without comparing the patch version. Because RPC serializes the numeric op value, an older peer can decode operations such as GGML_OP_SSM_CONV as a different operation, producing an invalid graph or incorrect results; append the new value without shifting existing IDs or make the handshake reject incompatible peers.

Useful? React with 👍 / 👎.

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.

2 participants