fix: isolate few-shot sampler RNG and pool from shared and global state (#1307, #1309) - #1379
Open
Abelo9996 wants to merge 1 commit into
Open
Conversation
The sequential and balanced few-shot samplers leaked state that corrupts variance-seed reproducibility. Sequential (huggingface#1307): _init_fewshot_sampling_sequential rotated the list returned by task.fewshot_docs() in place. That list is the task's memoized pool, returned by reference, so the rotation mutated shared state and every cached seed aliased the same over-rotated list, so each seed after the first selected the wrong examples. Copy the pool with list(...) before rotating, mirroring the random path. Balanced (huggingface#1309): _init_fewshot_sampling_balanced seeded and used the global random module, and its label loop guarded with "if not next_label", which also fires on a present but falsy label (int 0, empty string), truncating the selection. Use a local random.Random(variance_seed) and stop only when next_label is None. random.Random(seed) yields the same sequence as random.seed(seed) on clean state, so selections are unchanged in the common case; the change fixes the leaky and falsy-label cases. Adds unit tests for both. Updates test_fewshot_sampler[sequential], whose previous assertion passed only because the in-place rotation mutated the shared pool that the assertion also read from. Closes huggingface#1307 Closes huggingface#1309
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.
What
Two fixes in
FewShotSampler(src/lighteval/tasks/prompt_manager.py) that leak state and break variance-seed reproducibility.Sequential (#1307)
_init_fewshot_sampling_sequentialrotated the list returned bytask.fewshot_docs()in place. That list is the task's memoized_fewshot_docs, handed back by reference, so the rotation mutated shared state and each cached seed aliased the same over-rotated list. Withfew_shot_iterations > 1, offsets accumulate and every seed after the first selects the wrong examples. Fix: copy the pool withlist(...)before rotating, the same way_init_fewshot_sampling_randomalready does.Balanced (#1309)
_init_fewshot_sampling_balancedhad two issues:randommodule, so selection depended on and perturbed global RNG state. Fixed with a localrandom.Random(variance_seed).if not next_label, which also fires on a present but falsy label (int0, empty string), cutting the balanced selection short. Fixed to stop only onnext_label is None, which is the real "cycle exhausted" signal.Behaviour
random.Random(seed)produces the same sequence asrandom.seed(seed)on clean state, so selections are unchanged in the common case. The change fixes the cases where shared or global state leaked between seeds, and where a falsy label truncated the balanced sample.Tests
tests/unit/prompt/test_fewshot_sampler.py: sequential does not mutate the shared pool, sequential seeds are independent, a falsy label does not truncate the balanced sample, and balanced leaves global RNG state untouched.test_fewshot_sampler[sequential]intests/unit/prompt/test_prompt_manager.py: its previous assertion passed only because the in-place rotation mutated the shared pool the assertion also read from; it now checks the intended rotation ([20:40]).tests/unit/prompt/is green;ruff checkandruff format --checkpass on the changed files.Closes #1307
Closes #1309