Repository navigation
pr-set : repin Inkling, DiffusionGemma, #251 and #264 for b12164, drop #287 - #351
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…(in upstream as ggml-org#31679)
danielhanchen
changed the title
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.
The prebuilt is an aged upstream ggml-org
b####tag plus the pins inscripts/unsloth/pr-set.json, merged in order (git diff3, thenadditive_merge.pyfor pure add/add conflicts only). Since b12128-mix-2f23ac9, pins stopped merging onto the tags the next nightlies can pick, first at b12136 to b12144 and again at b12164. Each fix is aFix merge conflictsmerge of the oldest upstream commit or tag that carries the breaking change into the pin's own branch, keeping upstream verbatim and re-applying only the pin's intent.Pins
--moe-cache-mib autocommon_get_device_memory_data_impltakes the skipped NextN layer countnskargument added to the pin's calls, including one pin-only call in the MoE cache loop that merged cleanly but would not have compiled; the pin'sadd_extra_memory(.., mparams)and shared-modelmparamskept184123bconversion/base.py), ggml-org#31047 (b12144, MoE layout for MMQ/MMF inmmq.cuh), ggml-org#31683 and the new model structs inmodels.h(b12164)3258a1d): Inkling's wrong-shard check before upstream's_source_tensor_namesrecording;mmq.cuhis upstream's file with Inkling's int64 dst-offset widening re-applied, nothing else. Then merge b12164: upstream'smodels.hverbatim plus only thellama_model_inklingstructf4b5613constants.py,llama-arch.cpp/.handmodels.h(ggml-org#31589, ggml-org#31692, ggml-org#31698, ggml-org#31683, b12164)diffusion.*keys and model struct6745aa6mmq.cuhnow), the pin's device expert sort, GET_ROWS kernel and dispatch kept on top9fec7cbuncheckedentry infeature-checks.jsonThe Inkling and DiffusionGemma PR branches carry the new pins through merges with no content change, so each pin is a commit of its PR. #251 and #264 are retargeted to
base/upstream-<sha9>branches, so each PR's diff is again only its own change.Evidence
Merge loop +
pin_contract.py+merge_checks.py(local sim of this exactpr-set.json): all 25 pins merge and are intact on b12144, b12167, b12170 (the aged base of the next scheduled nightly), b12171 and b12172 (the newest tag), andmerge_checksis clean on each. Full CPU build of every target on b12167: 132 binaries, 0 errors.GPU, same checks as
llama_pinset.py verify --cudaon the merged tree (tree hash checked against the local merge before testing), withtest-backend-opsalso run on the plain tag:Neither side has a failure; the pin set runs more cases than the plain tag because pins add types and ops.
cuda-moe-prompt(#264) matches the CPU reference in 1027/1027 MUL_MAT_ID and 349/349 GET_ROWS cases.Numeric gates (CPU,
-ngl 0):unsloth/Inkling-Small-GGUFUD-IQ1_S, wikitext-2-c 512 --chunks 20:3258a1dandf4b5613both score byte-identical to the old pin48f7e71against itself across the whole result block (PPL 39.047836, mean KLD 0, same top-1 99.980%, which is also the A/A floor on CPU).unsloth/diffusiongemma-26B-A4B-it-GGUFQ4_K_M,llama-diffusion-cli -no-cnv -s 42 -n 256:6745aa6produces output identical to the old pin40faaef(twice each).test-backend-ops -b CUDA0, every arm with 0 failures and 0 new failures vs plain b12144: MUL_MAT_ID 984/984, MUL_MAT 2517/2517, GET_ROWS 340/340, for the new pin with the counting sort forced on (GGML_CUDA_MM_IDS_SORT=1) and off (=0), and for the old pin on its own base.test-arg-parserpasses; the fit change has no numeric surface.#264 timing (MUL_MAT_ID, top-8): on L4 the new pin is 0.97x to 1.02x of plain b12144 (one repeat), so it is performance neutral after the port. ggml-org#31047 already carries most of the original pin's prompt speedup: on T4 the original pin on its old base is 1.09x of plain b12144 (dense control 1.025x, 3 repeats). Whether to keep #264 in the set is a separate follow-up.
Not covered