Skip to content

fix(sampling): preserve cached choices during scoring - #10138

Open
Excelius-Wang wants to merge 2 commits into
modelscope:mainfrom
Excelius-Wang:fix/sampling-cache-choices
Open

Excelius-Wang wants to merge 2 commits into
modelscope:mainfrom
Excelius-Wang:fix/sampling-cache-choices

Conversation

@Excelius-Wang

Copy link
Copy Markdown
Contributor

PR type

  • Bug Fix
  • New Feature
  • Document Updates
  • More Models or Datasets Support

PR information

When sampling from a valid cache, generate returns the cached choices list. do_sample appends the ground truth to that same list, changing the cache's answer count. Reusing the cache key then misses the exact-count check and attempts generation; with --sampler_engine no, this raises AttributeError: 'NoneType' object has no attribute 'infer'. Duplicate rows within a batch also share the mutated list.

Copy the choices list before adding the ground truth. This preserves the cache while reusing the existing scoring and output logic.

Experiment results

CUDA_VISIBLE_DEVICES='' OMP_NUM_THREADS=1 HF_HUB_OFFLINE=1 \
  PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest \
  tests/general/test_sampler_cache.py \
  tests/general/test_sampler_engine_kwargs.py -q
pre-commit run --all-files
pre-commit run isort --all-files
  • 19 tests pass, covering repeated calls, duplicate rows within a batch, cached and generated choices, ORM/PRM filtering, unchanged inputs/cache, and output contents.
  • Before the fix, all four cached single-row regression cases fail on the second call; the four uncached controls pass.
  • A separate CPU probe generated a cache through SwiftSampling.run, then reused it with the original dataset sampled twice via #2. The upstream method fails on the second row; the fixed method writes both scored rows and preserves the cache.
  • All pre-commit hooks pass.

Sampling and cache methods run natively; model outputs and reward scores are mocked. Full model inference and a live Ray cluster were not run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant