Repository navigation
[vllm, rollout] fix: handle non-contiguous weights in bucketed transfer - #6919
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function _weight_to_bytes to handle non-contiguous (strided) tensors when converting weights to bytes, along with a corresponding unit test. The reviewer suggests optimizing this process by directly copying the weight into a reshaped view of the destination buffer, which avoids the temporary GPU memory allocation and extra copy overhead caused by calling .contiguous().
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "handle": None, | ||
| } | ||
| self.buffer[offset : offset + weight.nbytes].copy_(weight.view(-1).view(torch.uint8), non_blocking=True) | ||
| self.buffer[offset : offset + weight.nbytes].copy_(_weight_to_bytes(weight), non_blocking=True) |
There was a problem hiding this comment.
Instead of making a contiguous copy of the non-contiguous weight tensor via _weight_to_bytes(weight) (which calls weight.contiguous()), we can avoid any temporary GPU memory allocations and extra copy overhead by viewing the destination buffer slice with the target dtype and shape, and copying the weight directly.
PyTorch's copy_ natively supports copying from non-contiguous/strided source tensors to contiguous destination tensors.
Why this is better:
- Zero Temporary Allocations:
weight.contiguous()allocates a temporary tensor of the same size asweighton the GPU. For large models, this can cause GPU memory spikes and potential Out-Of-Memory (OOM) errors. - Fewer Copies: It reduces the operation from two copies (
weight-> temporary contiguous tensor ->self.buffer) to a single direct copy (weight->self.buffer).
| self.buffer[offset : offset + weight.nbytes].copy_(_weight_to_bytes(weight), non_blocking=True) | |
| self.buffer[offset : offset + weight.nbytes].view(dtype=weight.dtype).view(weight.shape).copy_(weight, non_blocking=True) |
@Mecoli1219 In which case is parameter non-contiguous? I never meet this error before. |
|
I met this error when using megatron as the trainer via megatron-bridge v0.5.0. I am suspecting the gate tensors here is the one that causing the non-continuous tensor, but need to verify it though. |
…er (verl-project#6919) ### What does this PR do? Fixes vLLM rollout weight updates when the actor exports a valid but non-contiguous tensor. The bucketed weight sender previously packed weights with `weight.view(-1).view(torch.uint8)`, which crashes for strided tensors with: ```text RuntimeError: view size is not compatible with input tensor's size and stride ``` This PR adds a small byte-packing helper that first compacts the tensor with `.contiguous()` before reinterpreting it as `torch.uint8`. This keeps the common contiguous path cheap while allowing exporters such as Megatron-Bridge to return strided tensor views. ### Checklist Before Starting - [x] Search for similar PRs. Paste at least one query link here: - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+bucketed_weight_transfer - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+non-contiguous+tensor+update_weights - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+view+size+is+not+compatible+input+tensor+stride - [x] Format the PR title as `[{modules}] {type}: {description}` (This will be checked by the CI) - `{modules}` include `fsdp`, `megatron`, `veomni`, `sglang`, `vllm`, `rollout`, `trainer`, `ci`, `training_utils`, `recipe`, `hardware`, `deployment`, `ray`, `worker`, `single_controller`, `misc`, `perf`, `model`, `algo`, `env`, `tool`, `ckpt`, `doc`, `data`, `cfg`, `reward`, `fully_async`, `one_step_off` - If this PR involves multiple modules, separate them with `,` like `[megatron, fsdp, doc]` - `{type}` is in `feat`, `fix`, `refactor`, `chore`, `test` - If this PR breaks any API (CLI arguments, config, function signature, etc.), add `[BREAKING]` to the beginning of the title. - Example: `[BREAKING][fsdp, megatron] feat: dynamic batching` ### Test - Added a unit test for it. - Tested with the latest megatron-bridge v0.5.0, and verified it is working. ### API and Usage Example No API or user-facing usage changes. ### Design & Code Changes - Added a helper who calls `weight.contiguous().view(-1).view(torch.uint8)` so valid strided/non-contiguous tensors can be serialized into the contiguous bucket buffer. ### Checklist Before Submitting > [!IMPORTANT] > Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review. - [x] Read the [Contribute Guide](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md). - [ ] Apply [pre-commit checks](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md#code-linting-and-formatting): `pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always` - [ ] Add / Update [the documentation](https://github.com/verl-project/verl/tree/main/docs). - [ x Add unit or end-to-end test(s) to [the CI workflow](https://github.com/verl-project/verl/tree/main/.github/workflows) to cover all the code. If not feasible, explain why: ... - [ ] Once your PR is ready for CI, send a message in [the `ci-request` channel](https://verl-project.slack.com/archives/C091TCESWB1) in [the `verl` Slack workspace](https://join.slack.com/t/verl-project/shared_invite/zt-3855yhg8g-CTkqXu~hKojPCmo7k_yXTQ). (If not accessible, please try [the Feishu group (飞书群)](https://applink.larkoffice.com/client/chat/chatter/add_by_link?link_token=772jd4f1-cd91-441e-a820-498c6614126a).) - [ ] If your PR is related to the `recipe` submodule, please also update the reference to the submodule commit via `git submodule update --remote` or `cd recipe && git pull origin main`.
…#6919) ### What does this PR do? Fixes vLLM rollout weight updates when the actor exports a valid but non-contiguous tensor. The bucketed weight sender previously packed weights with `weight.view(-1).view(torch.uint8)`, which crashes for strided tensors with: ```text RuntimeError: view size is not compatible with input tensor's size and stride ``` This PR adds a small byte-packing helper that first compacts the tensor with `.contiguous()` before reinterpreting it as `torch.uint8`. This keeps the common contiguous path cheap while allowing exporters such as Megatron-Bridge to return strided tensor views. ### Checklist Before Starting - [x] Search for similar PRs. Paste at least one query link here: - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+bucketed_weight_transfer - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+non-contiguous+tensor+update_weights - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+view+size+is+not+compatible+input+tensor+stride - [x] Format the PR title as `[{modules}] {type}: {description}` (This will be checked by the CI) - `{modules}` include `fsdp`, `megatron`, `veomni`, `sglang`, `vllm`, `rollout`, `trainer`, `ci`, `training_utils`, `recipe`, `hardware`, `deployment`, `ray`, `worker`, `single_controller`, `misc`, `perf`, `model`, `algo`, `env`, `tool`, `ckpt`, `doc`, `data`, `cfg`, `reward`, `fully_async`, `one_step_off` - If this PR involves multiple modules, separate them with `,` like `[megatron, fsdp, doc]` - `{type}` is in `feat`, `fix`, `refactor`, `chore`, `test` - If this PR breaks any API (CLI arguments, config, function signature, etc.), add `[BREAKING]` to the beginning of the title. - Example: `[BREAKING][fsdp, megatron] feat: dynamic batching` ### Test - Added a unit test for it. - Tested with the latest megatron-bridge v0.5.0, and verified it is working. ### API and Usage Example No API or user-facing usage changes. ### Design & Code Changes - Added a helper who calls `weight.contiguous().view(-1).view(torch.uint8)` so valid strided/non-contiguous tensors can be serialized into the contiguous bucket buffer. ### Checklist Before Submitting > [!IMPORTANT] > Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review. - [x] Read the [Contribute Guide](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md). - [ ] Apply [pre-commit checks](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md#code-linting-and-formatting): `pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always` - [ ] Add / Update [the documentation](https://github.com/verl-project/verl/tree/main/docs). - [ x Add unit or end-to-end test(s) to [the CI workflow](https://github.com/verl-project/verl/tree/main/.github/workflows) to cover all the code. If not feasible, explain why: ... - [ ] Once your PR is ready for CI, send a message in [the `ci-request` channel](https://verl-project.slack.com/archives/C091TCESWB1) in [the `verl` Slack workspace](https://join.slack.com/t/verl-project/shared_invite/zt-3855yhg8g-CTkqXu~hKojPCmo7k_yXTQ). (If not accessible, please try [the Feishu group (飞书群)](https://applink.larkoffice.com/client/chat/chatter/add_by_link?link_token=772jd4f1-cd91-441e-a820-498c6614126a).) - [ ] If your PR is related to the `recipe` submodule, please also update the reference to the submodule commit via `git submodule update --remote` or `cd recipe && git pull origin main`.
…#6919) ### What does this PR do? Fixes vLLM rollout weight updates when the actor exports a valid but non-contiguous tensor. The bucketed weight sender previously packed weights with `weight.view(-1).view(torch.uint8)`, which crashes for strided tensors with: ```text RuntimeError: view size is not compatible with input tensor's size and stride ``` This PR adds a small byte-packing helper that first compacts the tensor with `.contiguous()` before reinterpreting it as `torch.uint8`. This keeps the common contiguous path cheap while allowing exporters such as Megatron-Bridge to return strided tensor views. ### Checklist Before Starting - [x] Search for similar PRs. Paste at least one query link here: - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+bucketed_weight_transfer - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+non-contiguous+tensor+update_weights - https://github.com/verl-project/verl/pulls?q=is%3Apr+is%3Aopen+view+size+is+not+compatible+input+tensor+stride - [x] Format the PR title as `[{modules}] {type}: {description}` (This will be checked by the CI) - `{modules}` include `fsdp`, `megatron`, `veomni`, `sglang`, `vllm`, `rollout`, `trainer`, `ci`, `training_utils`, `recipe`, `hardware`, `deployment`, `ray`, `worker`, `single_controller`, `misc`, `perf`, `model`, `algo`, `env`, `tool`, `ckpt`, `doc`, `data`, `cfg`, `reward`, `fully_async`, `one_step_off` - If this PR involves multiple modules, separate them with `,` like `[megatron, fsdp, doc]` - `{type}` is in `feat`, `fix`, `refactor`, `chore`, `test` - If this PR breaks any API (CLI arguments, config, function signature, etc.), add `[BREAKING]` to the beginning of the title. - Example: `[BREAKING][fsdp, megatron] feat: dynamic batching` ### Test - Added a unit test for it. - Tested with the latest megatron-bridge v0.5.0, and verified it is working. ### API and Usage Example No API or user-facing usage changes. ### Design & Code Changes - Added a helper who calls `weight.contiguous().view(-1).view(torch.uint8)` so valid strided/non-contiguous tensors can be serialized into the contiguous bucket buffer. ### Checklist Before Submitting > [!IMPORTANT] > Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review. - [x] Read the [Contribute Guide](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md). - [ ] Apply [pre-commit checks](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md#code-linting-and-formatting): `pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always` - [ ] Add / Update [the documentation](https://github.com/verl-project/verl/tree/main/docs). - [ x Add unit or end-to-end test(s) to [the CI workflow](https://github.com/verl-project/verl/tree/main/.github/workflows) to cover all the code. If not feasible, explain why: ... - [ ] Once your PR is ready for CI, send a message in [the `ci-request` channel](https://verl-project.slack.com/archives/C091TCESWB1) in [the `verl` Slack workspace](https://join.slack.com/t/verl-project/shared_invite/zt-3855yhg8g-CTkqXu~hKojPCmo7k_yXTQ). (If not accessible, please try [the Feishu group (飞书群)](https://applink.larkoffice.com/client/chat/chatter/add_by_link?link_token=772jd4f1-cd91-441e-a820-498c6614126a).) - [ ] If your PR is related to the `recipe` submodule, please also update the reference to the submodule commit via `git submodule update --remote` or `cd recipe && git pull origin main`.
What does this PR do?
Fixes vLLM rollout weight updates when the actor exports a valid but non-contiguous tensor. The bucketed weight sender previously packed weights with
weight.view(-1).view(torch.uint8), which crashes for strided tensors with:This PR adds a small byte-packing helper that first compacts the tensor with
.contiguous()before reinterpreting it astorch.uint8. This keeps the common contiguous path cheap while allowing exporters such as Megatron-Bridge to return strided tensor views.Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI){modules}includefsdp,megatron,veomni,sglang,vllm,rollout,trainer,ci,training_utils,recipe,hardware,deployment,ray,worker,single_controller,misc,perf,model,algo,env,tool,ckpt,doc,data,cfg,reward,fully_async,one_step_off,like[megatron, fsdp, doc]{type}is infeat,fix,refactor,chore,test[BREAKING]to the beginning of the title.[BREAKING][fsdp, megatron] feat: dynamic batchingTest
API and Usage Example
No API or user-facing usage changes.
Design & Code Changes
weight.contiguous().view(-1).view(torch.uint8)so valid strided/non-contiguous tensors can be serialized into the contiguous bucket buffer.Checklist Before Submitting
Important
Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysci-requestchannel in theverlSlack workspace. (If not accessible, please try the Feishu group (飞书群).)recipesubmodule, please also update the reference to the submodule commit viagit submodule update --remoteorcd recipe && git pull origin main.