Repair C++ logit validation - #4
Open
kenjorissen wants to merge 1 commit into
Open
Conversation
validate-logits.py documented a C++ comparison but returned None without invoking the existing dump-logits binary. The binary also used std::partial_sort without including algorithm, which fails a clean GCC 15 build. Read dump-logits shape and float data, validate the serialized dimensions, and compare the result as documented. Cover the wrapper protocol with a deterministic stand-in executable, then exercise the real dump-logits binary and synthetic GGUF in the existing end-to-end test. Assisted-by: OpenAI Codex
kenjorissen
marked this pull request as ready for review
August 13, 2026 15:04
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
1 participant
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.
Summary
validate-logits.pyinvoke the existingdump-logitsexecutable and read its binary output<algorithm>include required bystd::partial_sortRoot cause
run_cpp()documented a C++ comparison but returnedNonewithout invokingdump-logits. The executable also depended on a transitive<algorithm>include, so a clean GCC 15 tools build failed atstd::partial_sort.This change makes the existing validation path operational and checks malformed or inconsistent output before comparison.
Validation
ctest --test-dir build --output-on-failure(2/2 passed)python3 -m py_compile tools/validate-logits.py tests/test-validate-logits.py tests/test-e2e.pyfloat32logits with shape[4, 152064]git diff --check upstream/main...HEADFollow-up context
I have separate fixes in preparation for Dream candidate ranking, Dream shifted-cache behavior, and tokenizer-specific mask removal. Their full tools builds depend on the missing-header repair here, and this working logit comparison provides a useful correctness check; they are intentionally excluded from this focused PR and will be rebased after it lands.
A larger, separate series adds GGML device discovery and CPU/GPU offload support, including cached and autoregressive paths plus backend-divergence diagnostics for Vulkan and Metal. That work will remain separate and build on the corrected CPU/validation baseline rather than expanding this PR.
AI assistance disclosure
I used OpenAI Codex to help investigate the bug, implement the change, and develop the tests. I reviewed the final diff and test results and take responsibility for the contribution.