Repository navigation
rust: return the dictionary parameters that training selected - #378
Open
pylaterreur wants to merge 1 commit into
Open
pylaterreur wants to merge 1 commit into
pylaterreur wants to merge 1 commit into
Conversation
train_dictionary() passes its ZDICT_fastCover_params_t to
ZDICT_optimizeTrainFromBuffer_fastCover(), which writes the parameters
it selected back into the struct. But the struct was passed through a
shared reference cast to *mut. Writing through such a pointer is
undefined behavior: the compiler marks the pointer readonly, and release
builds return the k and d values that were passed in. With the default
k=0, ZstdCompressionDict.k was 0, and d was 0 whenever d was searched.
test_optimize has been failing this way in every "Test Rust Backend" CI
step ("AssertionError: 0 not found in (50, 2000)").
Pass a mutable reference instead, as the C backend passes ¶ms, so k
and d match what the C and CFFI backends return. test_optimize passes
d=6, so add test_optimize_d to also check a searched d.
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN
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.
In the Rust backend,
train_dictionary()returns thekanddit was called with, not the onesZDICT_optimizeTrainFromBuffer_fastCover()selected. With the defaultk=0,ZstdCompressionDict.kis 0, anddis 0 when it is searched. The dictionary data itself is correct. This is thetest_optimizefailure (AssertionError: 0 not found in (50, 2000)) in every "Test Rust Backend" step of test.yml, e.g. all 41 Rust steps of the 2026-11-06 scheduled run. The step hascontinue-on-error: true, so the jobs stay green.zstd writes the selected parameters back into the params struct, but the struct was passed through a shared reference cast to
*mut, so the compiler treats it as unchanged by the call. The release build stores the originalk/din the result (the call passes%paramsasreadonlyin the LLVM IR); a debug build ofmainreturns the right values. This passes a mutable reference instead, as the C backend passes¶ms.test_optimizepassesd=6, sotest_optimize_dalso checks a searchedd.Rust backend before/after on Linux x86_64 with GitHub's CPython builds and test.yml's settings (uv 0.8.11,
ZSTD_SLOW_TESTS=1,ZSTD_WARNINGS_AS_ERRORS=1, hypothesisciprofile,PYTHONDEVMODE=1,-n 2):On my fork, with the Rust step's
continue-on-errorremoved so that a failure would fail the job, test.yml passes in all 48 jobs, and all 41 Rust steps report 293 passed. I leftcontinue-on-erroralone here.Prepared with the help of Claude Code.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN