Skip to content

llama: remove the unused qwen4exp mem_size - #120

Closed
oobabooga wants to merge 1 commit into
unslothai:qwen4exp-27742-b10632from
oobabooga:fix/pr114-macos-unused-variable
Closed

llama: remove the unused qwen4exp mem_size#120
oobabooga wants to merge 1 commit into
unslothai:qwen4exp-27742-b10632from
oobabooga:fix/pr114-macos-unused-variable

Conversation

@oobabooga

Copy link
Copy Markdown
Member

Follow-up to #114.

Problem

Both macOS legs of the nightly full release run failed to compile, so the publish job stopped at the build fan-in and nothing was released. Every other build leg passed.

src/models/qwen4exp.cpp:1027:16: error: unused variable 'mem_size' [-Werror,-Wunused-variable]
 1027 |     const auto mem_size = mctx_cur->get_size();

build_conv_state_at assigns mem_size and never reads it. It is the only occurrence in the file, and the shared build_conv_state this helper is modelled on does not use it either, so it is leftover from an older shape of that code.

Only unsloth-prebuilt-macos.yml passes -DLLAMA_FATAL_WARNINGS=ON. The CPU, CUDA, ROCm, Vulkan and Windows legs compiled the same warning and shipped, which is why one dead line takes down the release through macOS alone.

Change

Delete the declaration. Nothing else.

Verification

A/B on an Apple silicon Mac, building the llama target from the PR head at ca5d0a1 with LLAMA_FATAL_WARNINGS=ON:

  • Control at ca5d0a1: fails at qwen4exp.cpp:1027 with -Werror,-Wunused-variable.
  • Treatment with only this line removed: qwen4exp.cpp compiles and libllama links.

The reproduction narrowed the CI configuration in two ways, neither of which touches the failing compile: it built the llama target rather than the full release set, skipping BoringSSL and the server, and it used AppleClang 15 with Metal off, matching the macOS x64 leg. The arm64 leg's AppleClang 21 and Metal path were not exercised locally. The error is compiler-version independent and the CI logs show both legs failing identically.

Note for the nightly

The prebuild tree is the upstream tag plus exact commit pins, so this branch changes nothing until scripts/unsloth/pr-set.json is repinned. #118 is three commits ahead of #114 and does not touch qwen4exp.cpp, so it needs the corrected head merged in and its own pin updated. The same line is still present in ggml-org#27742, so a later refresh of the carry will reintroduce it unless it is fixed upstream too.

The pin preflight only dry-runs the merges and never compiles, so it could not have caught this.

@oobabooga

Copy link
Copy Markdown
Member Author

Closing as superseded.

@oobabooga oobabooga closed this Aug 27, 2026
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.

1 participant