You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hi, I rechecked this against current main. The duplicate work is still present: filter_long_prompt() tokenizes retained text prompts only to test their length, discards those IDs, and _prepare_prompt_ids() encodes the same rendered prompt again during the default rollout. #1113 improved tokenization correctness but did not retain or reuse those IDs.
I can submit a focused CPU-tested fix that caches only retained text prompt IDs when max-length filtering is enabled and consumes that cache in the default rollout path. I would intentionally leave multimodal processor outputs uncached: retaining pixel tensors for the whole Dataset and deepcopying them for every sampled sibling would create a much larger host-memory regression.
Would this text-only, default-rollout scope be the preferred first PR, or would you prefer a compact cache representation/API that custom rollout functions can explicitly opt into? I will keep max_length=None, custom rollout, and multimodal behavior unchanged.
Hi, I rechecked this against current main. The duplicate work is still present: filter_long_prompt() tokenizes retained text prompts only to test their length, discards those IDs, and _prepare_prompt_ids() encodes the same rendered prompt again during the default rollout. #1113 improved tokenization correctness but did not retain or reuse those IDs.
I can submit a focused CPU-tested fix that caches only retained text prompt IDs when max-length filtering is enabled and consumes that cache in the default rollout path. I would intentionally leave multimodal processor outputs uncached: retaining pixel tensors for the whole Dataset and deepcopying them for every sampled sibling would create a much larger host-memory regression.
Would this text-only, default-rollout scope be the preferred first PR, or would you prefer a compact cache representation/API that custom rollout functions can explicitly opt into? I will keep max_length=None, custom rollout, and multimodal behavior unchanged.
Activity
liuhao-labs commentedon Aug 23, 2026
Hi, I rechecked this against current
main. The duplicate work is still present:filter_long_prompt()tokenizes retained text prompts only to test their length, discards those IDs, and_prepare_prompt_ids()encodes the same rendered prompt again during the default rollout. #1113 improved tokenization correctness but did not retain or reuse those IDs.I can submit a focused CPU-tested fix that caches only retained text prompt IDs when max-length filtering is enabled and consumes that cache in the default rollout path. I would intentionally leave multimodal processor outputs uncached: retaining pixel tensors for the whole
Datasetand deepcopying them for every sampled sibling would create a much larger host-memory regression.Would this text-only, default-rollout scope be the preferred first PR, or would you prefer a compact cache representation/API that custom rollout functions can explicitly opt into? I will keep
max_length=None, custom rollout, and multimodal behavior unchanged.nanjiangwill commentedon Aug 23, 2026
replied in #2309 (comment)