[feat] GenRL: add runtime and memory stability helpers - #1402
Conversation
Extracted from hao-ai-lab#1391. GenRL-Stack: 3/6
|
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements and fixes across training scripts, EMA callbacks, evaluation, and sampling logic. Key changes include switching to module-based execution in run scripts, adding an update interval to the EMA callback, introducing deterministic seeding using Blake2b hashing, and adding support for synchronous reward scoring with CPU offloading. The review feedback highlights three main improvement opportunities: clamping std_scale to math.sqrt(tiny) instead of tiny in sde.py to prevent underflow during squaring, moving the expensive torch.cuda.empty_cache() call outside the loop in sampling.py, and instantiating the torch.Generator outside the evaluation loop in evaluation.py to reduce overhead.
| std_scale = torch.clamp( | ||
| std_scale, | ||
| min=torch.finfo(std_scale.dtype).tiny, | ||
| ) |
There was a problem hiding this comment.
Clamping std_scale to torch.finfo(std_scale.dtype).tiny is insufficient to prevent division by zero because std_scale**2 will underflow to 0.0 for any std_scale < sqrt(tiny). For float32, tiny is 1.17e-38, so any std_scale below 1.08e-19 will cause std_scale**2 to underflow to 0.0, leading to division by zero (NaN or inf).
Additionally, extremely small values of std_scale can cause severe numerical instability (exploding log-probabilities) if there is even a tiny discrepancy between prev_sample and prev_sample_mean due to precision limits.
It is safer to clamp std_scale to math.sqrt(torch.finfo(std_scale.dtype).tiny) or a small epsilon like 1e-5 to ensure numerical stability.
| std_scale = torch.clamp( | |
| std_scale, | |
| min=torch.finfo(std_scale.dtype).tiny, | |
| ) | |
| std_scale = torch.clamp( | |
| std_scale, | |
| min=math.sqrt(torch.finfo(std_scale.dtype).tiny), | |
| ) |
| if async_reward_scoring: | ||
| rewards, _ = sample["rewards"].result() | ||
| else: | ||
| videos_cpu, prompts, prompt_metadata = sample["rewards"] | ||
| torch.cuda.empty_cache() | ||
| rewards, _ = reward_fn( | ||
| videos_cpu, prompts, prompt_metadata, True | ||
| ) |
There was a problem hiding this comment.
Calling torch.cuda.empty_cache() inside the loop for every sample when async_reward_scoring is False is highly inefficient. empty_cache() is an expensive operation that forces CUDA synchronization and releases cached memory back to the OS, which can significantly slow down the epoch processing.
Instead, you should call torch.cuda.empty_cache() once before the loop starts (e.g., right after _t_reward_wait = time.perf_counter()) if not async_reward_scoring, and remove it from inside the loop.
if async_reward_scoring:
rewards, _ = sample["rewards"].result()
else:
videos_cpu, prompts, prompt_metadata = sample["rewards"]
rewards, _ = reward_fn(
videos_cpu, prompts, prompt_metadata, True
)| generator = torch.Generator(device=device) | ||
| generator.manual_seed(seed + batch_idx) |
There was a problem hiding this comment.
Instantiating a new torch.Generator(device=device) inside the loop for every evaluation batch introduces unnecessary overhead, especially on CUDA devices.
To optimize performance, instantiate the generator once outside the loop (e.g., right before for batch_idx, ... in enumerate(test_dataloader):) and simply re-seed it inside the loop using generator.manual_seed(seed + batch_idx).
Co-authored-by: Davids048 <jundasu@ucsd.edu>
Extracted from #1391.
GenRL-Stack: 3/6
Purpose
Add focused runtime stability helpers for GenRL sampling, evaluation, EMA cadence, launch entrypoints, and numerical SDE behavior.
This keeps the memory/runtime plumbing separate from reward compatibility, PPO loop changes, LoRA support, and recipe defaults.
Fixes #
Changes
-m fastvideo.train.entrypoint.trainin shell scripts.Test Plan
Test Results
Test output