llama: remove the unused qwen4exp mem_size - #120
Closed
oobabooga wants to merge 1 commit into
Closed
Conversation
Member
Author
|
Closing as superseded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
build_conv_state_atassignsmem_sizeand never reads it. It is the only occurrence in the file, and the sharedbuild_conv_statethis helper is modelled on does not use it either, so it is leftover from an older shape of that code.Only
unsloth-prebuilt-macos.ymlpasses-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
llamatarget from the PR head atca5d0a1withLLAMA_FATAL_WARNINGS=ON:ca5d0a1: fails atqwen4exp.cpp:1027with-Werror,-Wunused-variable.qwen4exp.cppcompiles andlibllamalinks.The reproduction narrowed the CI configuration in two ways, neither of which touches the failing compile: it built the
llamatarget 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.jsonis repinned. #118 is three commits ahead of #114 and does not touchqwen4exp.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.