Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the argument parsing and validation logic in vime/utils/arguments.py to clarify and document the behavior of the --offload-train option when --colocate is enabled. Specifically, it updates the help text and adds a detailed code comment explaining that while training offloading defaults to true under colocation, an explicit --no-offload-train flag is honored if both the training state and rollout engines fit in device memory, which significantly improves step performance by skipping the memory-saver copy. There are no review comments to address, and I have no additional feedback to provide.
e5736a4 to
8a481d2
Compare
The help text for --offload-train says "This will always be true when --colocate is set", but the code only defaults it to true when it is unset, and an explicit --no-offload-train is honoured. The difference matters: with the offload on, the memory-saver copy dominates the step, and skipping it when the training state and the rollout engines both fit in device memory took the step from 18.85 s to 3.32 s on 1 GPU with Qwen3-0.6B (same workload, same vLLM memory budget, 7 steady rollouts each). Say what the flag actually does, and note the trade-off where the colocate default is applied. No behaviour change. Signed-off-by: 0z5a <0z5a@users.noreply.github.com> Signed-off-by: 0z5a <dezhen.lu@student.uni-tuebingen.de>
8a481d2 to
c1659ae
Compare
|
Superseded by #460, now merged into |
Incremental review: Files changed.
Base:
0z5a-branchat791f101e20db377b28e15f12a00863baaea2605f.Correct
--offload-trainhelp:--colocateenables offload when the argument is unset, while explicit--no-offload-trainis honored. Explain that a resident actor requires enough GPU memory for the rollout engine alongside training state.Comment/help changes only. Seven steady Qwen3-0.6B rollouts per configuration on one colocated GPU, with the same workload and vLLM memory budget:
--no-offload-trainThis measures an existing configuration choice, not acceleration caused by the help change. Default-selection policy is discussed in #442.