Skip to content
Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3b6c5c7
fix qwen3.5 mrope vlm model
eligotts Jun 14, 2026
8d533a3
fix(qwen3.5): handle adjacent VLM image groups
hubert-marek Jun 26, 2026
bfde708
Merge branch 'main' into fix/qwen35-mrope-model
hubert-marek Jun 26, 2026
febc12a
feat(trainer): pack RL multimodal samples
hubert-marek Jun 26, 2026
ba7a0c7
Packing implemenation support flag
hubert-marek Jun 26, 2026
b259fef
refactor(trainer): make seq_lens the packing boundary
hubert-marek Jun 26, 2026
fc3fac6
fix(trainer): remove unused packing variable
hubert-marek Jun 26, 2026
15956ad
Remove defensive
hubert-marek Jun 27, 2026
b8a212d
fix(trainer): validate multimodal packing support
hubert-marek Jun 27, 2026
dd099de
fix(trainer): clean up multimodal packing defaults
hubert-marek Jun 28, 2026
4b9cfd7
Merge branch 'main' into feat/rl-mm-packing
hubert-marek Jun 28, 2026
6a608da
fix(trainer): remove unused batch variable
hubert-marek Jun 28, 2026
beb66f4
fix(trainer): update multimodal packing validation tests
hubert-marek Jun 28, 2026
78bdaaa
fix(config): reject multimodal packing with cp
hubert-marek Jun 28, 2026
40237e1
fix(config): disable multimodal packing for cp configs
hubert-marek Jun 28, 2026
8e293c6
Enable Ulysses CP for Qwen VLM training
eligotts Jun 18, 2026
465c05c
Enable Ulysses CP for Qwen VLM with frozen vision encoder + embeddings
S1ro1 Jun 22, 2026
9f99604
WIP simplify multimodal CP packing
hubert-marek Jun 29, 2026
34b72e5
Simplify VLM CP dummy vision path
hubert-marek Jun 29, 2026
0b9e252
Merge main into VLM CP branch
hubert-marek Jun 29, 2026
24eee16
Tidy VLM CP PR cleanup
hubert-marek Jun 29, 2026
cc0d66f
Allow trainable embeddings with VLM CP
hubert-marek Jun 30, 2026
bd9c981
Move VLM CP prep into root forward
hubert-marek Jun 30, 2026
975b117
Fix VLM CP test lint
hubert-marek Jun 30, 2026
a973064
Guard VLM CP and preserve packed boundaries
hubert-marek Jun 30, 2026
7218a53
Stop leaking seq_lens to generic forwards
hubert-marek Jun 30, 2026
2015e71
Use explicit packed boundary model contract
hubert-marek Jun 30, 2026
c8cf6c9
Thread packed metadata through custom models
hubert-marek Jun 30, 2026
c8a1b96
Use model hook for packed forward metadata
hubert-marek Jun 30, 2026
600c062
Format packed metadata hook
hubert-marek Jun 30, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 41 additions & 37 deletions src/prime_rl/trainer/batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ def prepare_sample(training_example: TrainingSample, seq_len: int) -> MicroBatch
assert len(mm_token_type_ids) == len(input_ids), (
f"mm_token_type_ids: {len(mm_token_type_ids)}, input_ids: {len(input_ids)}"
)
if mm_kwargs is not None and "image_grid_thw" in mm_kwargs and mm_token_type_ids is None:
raise ValueError("image_grid_thw multimodal samples require mm_token_type_ids")
assert len(env_names) == len(input_ids), f"env_names: {len(env_names)}, input_ids: {len(input_ids)}"

return MicroBatch(
Expand All @@ -214,6 +216,7 @@ def prepare_sample(training_example: TrainingSample, seq_len: int) -> MicroBatch
rl_weights=rl_weights,
ce_weights=ce_weights,
ref_kl_weights=ref_kl_weights,
seq_lens=[len(input_ids)],
)


Expand All @@ -237,15 +240,14 @@ def first_sample(self) -> MicroBatch:

def can_add(self, sample: MicroBatch, max_seq_len: int) -> bool:
# Loss routing is per token (component weight streams), so samples of
# different loss types pack together freely — only modality, length and
# routed-experts presence constrain packing.
# different loss types pack together freely. The packer groups by
# run/LoRA before this point.
first_sample = self.first_sample
return (
not _is_multimodal_sample(first_sample)
and not _is_multimodal_sample(sample)
and self.length + len(sample.input_ids) <= max_seq_len
and (first_sample.routed_experts is None) == (sample.routed_experts is None)
)
if self.length + len(sample.input_ids) > max_seq_len:
return False
if (first_sample.routed_experts is None) != (sample.routed_experts is None):
return False
return True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid packing non-segment-aware multimodal samples

When two Qwen-style multimodal samples fit in one bin, this unconditional acceptance packs them into a single sequence. trainer.model.forward() drops the trainer's reset 2-D position_ids whenever image_grid_thw is present, and only the custom qwen3_5_moe path consumes seq_lens; HF VLM paths such as qwen3_vl recompute MRoPE for the concatenation as one sample, so positions and causal attention cross sample boundaries and corrupt multimodal training.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid packing generic multimodal prompts together

When a non-Qwen/generic VLM sample has mm_kwargs without image_grid_thw (for example the registered HF gemma4 path), this now allows multiple image prompts to be concatenated into one microbatch. That forward path only sends reset position_ids and no attention mask or seq_lens, so HF causal attention treats the packed tokens and images as one long prompt; later samples can attend to earlier samples and corrupt training targets. Keep multimodal packing limited to models that actually consume packed boundaries, or leave these samples as separate microbatches.

Useful? React with 👍 / 👎.


def add(self, lora_idx: int, sample: MicroBatch) -> None:
self.samples.append((lora_idx, sample))
Expand Down Expand Up @@ -290,6 +292,8 @@ def _materialize_bin(bin_content: _MicroBatchBin, num_loras: int) -> MicroBatch:
env_names: list[str] = []
ref_logprobs: list[float] | None = [] if has_ref_logprobs else None
mm_token_type_ids: list[int] | None = [] if has_mm_token_type_ids else None
mm_kwargs: dict[str, EncodedTensor] | None = None
seq_lens: list[int] = []
streams: dict[str, list[float] | None] = {name: ([] if has_stream[name] else None) for name in STREAM_FILL}
routed_experts: RoutedExperts | None = None
lora_num_tokens = [0] * num_loras
Expand Down Expand Up @@ -322,11 +326,19 @@ def _materialize_bin(bin_content: _MicroBatchBin, num_loras: int) -> MicroBatch:
assert routed_experts.shape[1:] == sample.routed_experts.shape[1:]
routed_experts.data += sample.routed_experts.data
routed_experts.shape[0] += sample.routed_experts.shape[0]
if sample.mm_kwargs is not None:
if mm_kwargs is None:
mm_kwargs = copy.deepcopy(sample.mm_kwargs)
else:
for key in mm_kwargs:
mm_kwargs[key].data += sample.mm_kwargs[key].data
mm_kwargs[key].shape[0] += sample.mm_kwargs[key].shape[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Packed MM kwargs key mismatch

Medium Severity

When merging packed multimodal samples, _materialize_bin only concatenates keys already present in the accumulated mm_kwargs. Extra keys on a later sample are dropped, and a missing key on a later sample causes KeyError during packing.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 24eee16. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not possible to happen under 1 model / 1 image processor

seq_lens.extend(sample.seq_lens)
lora_num_tokens[lora_idx] += sample_len

sequence_lengths = [len(sample.input_ids) for _, sample in bin_content.samples]
assert sum(sequence_lengths) == len(input_ids), (sequence_lengths, len(input_ids))
first_sample = bin_content.first_sample
assert sum(seq_lens) == len(input_ids), (seq_lens, len(input_ids))

return MicroBatch(
input_ids=input_ids,
Expand All @@ -341,10 +353,11 @@ def _materialize_bin(bin_content: _MicroBatchBin, num_loras: int) -> MicroBatch:
routed_experts=routed_experts,
mm_token_type_ids=mm_token_type_ids,
env_names=env_names,
mm_kwargs=first_sample.mm_kwargs if _is_multimodal_sample(first_sample) else None,
mm_kwargs=mm_kwargs,
rl_weights=streams["rl_weights"],
ce_weights=streams["ce_weights"],
ref_kl_weights=streams["ref_kl_weights"],
seq_lens=seq_lens,
)


Expand Down Expand Up @@ -374,19 +387,17 @@ def packed_samples_into_micro_bs(
) -> list[MicroBatch]:
"""
Pack samples into micro_batch efficiently.
We follow the First Fit Decreasing algorithm to pack the samples into bins and minimize potential padding while never truncating.
With per-token temperatures, samples can be packed together regardless of their temperature values.

NOTE: Multimodal samples (with mm_kwargs) are NOT packed together as they have variable-sized
vision data that doesn't pack well. Each multimodal sample becomes its own micro batch.
We follow the First Fit Decreasing algorithm to pack samples into bins and
minimize potential padding while never truncating. Packed batches preserve
sample boundaries in ``seq_lens``.
"""
# Sort by (lora_idx, -length) for packing efficiency
samples.sort(key=lambda x: (x[0], -len(x[1].input_ids)))

bins: list[_MicroBatchBin] = []

for idx, sample in samples:
# Try to find a bin that can fit this sequence (only pack text-only samples)
# Try to find a bin that can fit this sequence.
for bin_content in bins:
if bin_content.can_add(sample, max_seq_len):
bin_content.add(idx, sample)
Expand Down Expand Up @@ -464,6 +475,7 @@ def pad_micro_batch(micro_batch: MicroBatch, pad_to_multiple_of: int) -> MicroBa
)
if micro_batch.mm_token_type_ids is not None:
micro_batch.mm_token_type_ids.extend([0] * padding_size)
micro_batch.seq_lens.append(padding_size)
if micro_batch.routed_experts is not None:
_pad_routed_experts(micro_batch, padding_size)
micro_batch.env_names.extend([""] * padding_size)
Expand Down Expand Up @@ -538,32 +550,24 @@ def prepare_batch(
Prepare a batch of problems for each GPU. Each batch is a list of micro batches.
Each micro batch is shape [1, seq_len], the number of samples is not fixed per micro batch.

FSDP requires all ranks to execute the same operations at each step. If one rank
processes a multimodal batch (triggering the vision encoder) while another processes
a text-only batch, the all-gather will hang. We separate micro batches by modality
and distribute them so that at each step index, all ranks see the same modality.
VLM models handle text-only samples with a synthetic dummy vision path, so
microbatches can be distributed through one generic packing path.
Comment on lines +553 to +554

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep HF VLM batches modality-aligned

This assumption only holds for the custom Qwen3.5 VLM path; get_model still selects AutoModelForImageTextToText for registered VLMs without a custom class, such as Gemma/Qwen3-VL. In those HF VLM runs, text-only microbatches skip the FSDP-wrapped vision encoder while image microbatches enter it, and the combined distribution below can put those two modalities at the same micro-step on different ranks, recreating the vision-collective hang the deleted modality grouping prevented. Keep modality grouping for non-custom VLMs or add the dummy vision path there too.

Useful? React with 👍 / 👎.

"""
all_samples = [(idx, prepare_sample(rollout, seq_len)) for idx, rollout in zip(idxs, rollouts)]

micro_batches = packed_samples_into_micro_bs(all_samples, seq_len, num_loras, num_train_workers, bin_cost)
micro_batches = packed_samples_into_micro_bs(
all_samples,
seq_len,
num_loras,
num_train_workers,
bin_cost,
)
micro_batches = [pad_micro_batch(micro_batch, pad_to_multiple_of) for micro_batch in micro_batches]

# Separate by modality so each step index has uniform modality across all ranks
mm_batches = [b for b in micro_batches if _is_multimodal_sample(b)]
text_batches = [b for b in micro_batches if not _is_multimodal_sample(b)]
micro_batches = _pad_group_for_distribution(micro_batches, num_train_workers)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep VLM modality groups aligned across ranks

When the packed list contains separate image and text-only microbatches and num_train_workers > 1, distributing one combined pool can schedule an image microbatch on one rank and a text-only microbatch at the same step on another. For registered HF VLMs that do not have the new dummy-vision path (the custom VLM mapping only covers qwen3_5_moe), the text-only rank skips the FSDP-sharded vision encoder while the image rank enters it, reintroducing the unmatched all-gather/hang that the old mm/text grouping avoided.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do it on purpose - VLMs with CP need custom implementation

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep VLM microsteps modality-aligned

When VLM training uses an HF/generic VLM (for example the registered gemma4 path loaded through AutoModelForImageTextToText) and a step contains both text-only and image samples, this single mixed group can now be distributed so one data rank runs the vision encoder while another rank gets a text-only forward. Unlike the custom Qwen3.5 path, those HF text-only forwards do not run a dummy vision encoder, so FSDP ranks enter different vision-encoder collectives and can hang—the case the removed modality split was preventing. Keep modality grouping unless the selected model is known to execute a dummy vision path for text-only batches.

Useful? React with 👍 / 👎.

@hubert-marek hubert-marek Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VLM require custom implementations for CP


# Pad each group independently so its count is divisible by num_train_workers
mm_batches = _pad_group_for_distribution(mm_batches, num_train_workers)
text_batches = _pad_group_for_distribution(text_batches, num_train_workers)

# Alignment check after distribution padding so the dummy batches are covered too
for micro_batch in (*mm_batches, *text_batches):
# Alignment check after distribution padding so the dummy batches are covered too.
for micro_batch in micro_batches:
_assert_token_arrays_aligned(micro_batch)

batches_per_gpu: list[list[MicroBatch]] = [[] for _ in range(num_train_workers)]
for group in (mm_batches, text_batches):
group_batches_per_gpu = _distribute_group(group, num_train_workers, bin_cost)
for worker_idx, worker_batches in enumerate(group_batches_per_gpu):
batches_per_gpu[worker_idx].extend(worker_batches)

return batches_per_gpu
return _distribute_group(micro_batches, num_train_workers, bin_cost)
102 changes: 85 additions & 17 deletions src/prime_rl/trainer/model.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import inspect
import logging
import os
import time
Expand Down Expand Up @@ -56,7 +57,11 @@
from prime_rl.utils.logger import get_logger
from prime_rl.utils.sequence import get_cu_seqlens_from_position_ids
from prime_rl.utils.utils import format_time
from prime_rl.utils.vlm import get_language_model, get_vision_encoder, is_vlm_architecture
from prime_rl.utils.vlm import (
get_language_model,
get_vision_encoder,
is_vlm_architecture,
)


def pre_download_model(model_name: str) -> None:
Expand Down Expand Up @@ -476,6 +481,12 @@ def get_model(
raise ValueError(
"VLM models must use optimization_dtype='bfloat16' and reduce_dtype='bfloat16' to match vLLM inference."
)
# Context parallelism patches the global flash-attention dispatch to do the
# Ulysses all-to-all, which is causal- and sharded-sequence-only.
vision_config = getattr(model_config, "vision_config", None)
if config.cp > 1 and vision_config is not None:
vision_config._attn_implementation = "sdpa"
logger.info("CP enabled for VLM: pinning vision encoder attention to 'sdpa' (excluded from Ulysses CP).")

# GPT-OSS only supports FlashAttention via kernels-community/vllm-flash-attn3, which requires Hopper (SM 90).
# On other architectures (e.g. Blackwell), users must fall back to eager attention.
Expand Down Expand Up @@ -583,6 +594,12 @@ def get_model(
else:
impl_to_use = config.impl

if config.vlm is not None and config.cp > 1 and not (is_vlm_arch and impl_to_use == "custom" and custom_vlm_cls):
raise ValueError(
"VLM context parallelism requires a custom PrimeRL VLM implementation; "
f"{getattr(model_config, 'model_type', config.name)!r} is only supported with cp=1."
)

with device:
if impl_to_use == "custom" and custom_vlm_cls is not None:
model_cls = custom_vlm_cls
Expand Down Expand Up @@ -663,13 +680,31 @@ def setup_fsdp(model: nn.Module, config: ModelConfig, parallel_dims: ParallelDim
dp_mod_ep_mesh = parallel_dims.world_mesh[tuple(dp_mod_ep_mesh_dim_names)]

is_vlm_training = config.vlm is not None
cp_vlm = is_vlm_training and config.cp > 1
# Params kept out of FSDP entirely (full/replicated on every rank).
fsdp_ignored_params: set[nn.Parameter] = set()

def _all_params_frozen(module: nn.Module) -> bool:
params = list(module.parameters())
return bool(params) and not any(p.requires_grad for p in params)

def _ignore_params(module: nn.Module, name: str) -> None:
params = list(module.parameters())
if not params:
return
fsdp_ignored_params.update(params)
get_logger().info(f"CP+VLM: keeping frozen {name} out of FSDP (ignored_params).")

if is_vlm_training:
vision_encoder = get_vision_encoder(model, override=config.vlm.vision_encoder_attr)
if vision_encoder is None:
raise ValueError(f"VLM model {config.name} has no recognized vision encoder")

fully_shard(vision_encoder, mesh=hsdp_mesh, **fsdp_config)
get_logger().info(f"Applied FSDP to vision encoder (frozen={config.vlm.freeze_vision_encoder})")
if cp_vlm and _all_params_frozen(vision_encoder):
_ignore_params(vision_encoder, "vision encoder")
else:
fully_shard(vision_encoder, mesh=hsdp_mesh, **fsdp_config)
get_logger().info(f"Applied FSDP to vision encoder (frozen={config.vlm.freeze_vision_encoder})")

language_model = get_language_model(model, override=config.vlm.language_model_attr if is_vlm_training else None)
transformer_layers = language_model.layers
Expand All @@ -692,11 +727,12 @@ def setup_fsdp(model: nn.Module, config: ModelConfig, parallel_dims: ParallelDim
if shard_norm_and_lm_head:
# This optimization breaks weight tying
embed_module = getattr(language_model, "embed_tokens", None) or getattr(language_model, "embeddings", None)
fully_shard(
embed_module,
mesh=hsdp_mesh,
**fsdp_config,
)
if embed_module is not None:
fully_shard(
embed_module,
mesh=hsdp_mesh,
**fsdp_config,
)
norm_module = getattr(language_model, "norm", None) or language_model.norm_f
fully_shard(
[model.lm_head, norm_module],
Expand All @@ -714,6 +750,7 @@ def setup_fsdp(model: nn.Module, config: ModelConfig, parallel_dims: ParallelDim
mp_policy=mp_policy,
offload_policy=offload_policy,
reshard_after_forward=config.reshard_after_forward,
ignored_params=fsdp_ignored_params or None,
)

if not parallel_dims.ep_enabled:
Expand Down Expand Up @@ -1164,28 +1201,47 @@ def setup_model(
return model


def _model_accepts_seq_lens(model: nn.Module) -> bool:
accepts_seq_lens = getattr(model, "_prime_rl_accepts_seq_lens", None)
if accepts_seq_lens is None:
accepts_seq_lens = "seq_lens" in inspect.signature(model.forward).parameters
setattr(model, "_prime_rl_accepts_seq_lens", accepts_seq_lens)
return accepts_seq_lens


def forward(
model: nn.Module,
input_ids: Int[Tensor, "batch seq"],
position_ids: Int[Tensor, "batch seq"],
input_ids: Int[Tensor, "batch seq"] | None,
position_ids: Int[Tensor, "batch seq"] | None,
labels: Int[Tensor, "batch seq"] | None = None,
temperature: Tensor | None = None,
routed_experts: Int[Tensor, "batch seq layers topk"] | None = None,
inputs_embeds: Tensor | None = None,
# Generic multimodal kwargs (e.g. {"pixel_values": ...,
# "image_grid_thw": ...} for Qwen3-VL; just {"pixel_values": ...}
# for Gemma3). Passed straight through to ``model(**kwargs)`` so
# the model's HF forward signature is the schema. ``mm_token_type_ids``
# is split out because it's prime-rl-computed (from token ids),
# not a renderer/processor output.
# is split out because it's prime-rl-computed (from token ids).
mm_kwargs: dict[str, Tensor] | None = None,
mm_token_type_ids: Int[Tensor, "batch seq"] | None = None,
seq_lens: Int[Tensor, "segments"] | None = None,
seq_lens_are_global: bool = False,
cp_group: object | None = None,
cp_rank: int | None = None,
cp_world_size: int | None = None,
cp_style: str | None = None,
) -> PrimeLmOutput:
forwards_seq_lens = seq_lens is not None and _model_accepts_seq_lens(model)

# Build kwargs for model forward
kwargs = {
"input_ids": input_ids,
"labels": labels,
"temperature": temperature,
}
if inputs_embeds is None:
kwargs["input_ids"] = input_ids
else:
kwargs["inputs_embeds"] = inputs_embeds

if mm_kwargs:
# Forward the per-model multimodal tensors verbatim, plus the
Expand All @@ -1194,18 +1250,30 @@ def forward(
kwargs.update(mm_kwargs)
if mm_token_type_ids is not None:
kwargs["mm_token_type_ids"] = mm_token_type_ids
# ``position_ids`` for MRoPE families: Qwen3-VL's HF forward
# recomputes 3D positions from ``image_grid_thw`` and breaks if
# given the trainer's pre-computed 1D ``position_ids``. Detect
# via the mm_kwargs shape so we don't enumerate model_types.
if "image_grid_thw" not in mm_kwargs:
kwargs["position_ids"] = position_ids
elif forwards_seq_lens:
kwargs["seq_lens"] = seq_lens
else:
kwargs["position_ids"] = position_ids
if forwards_seq_lens:
kwargs["seq_lens"] = seq_lens
if seq_lens_are_global and forwards_seq_lens:
kwargs["seq_lens_are_global"] = True

if routed_experts is not None:
kwargs["routed_experts"] = routed_experts

if cp_group is not None:
kwargs.update(
{
"cp_group": cp_group,
"cp_rank": cp_rank,
"cp_world_size": cp_world_size,
"cp_style": cp_style,
}
)

out = model(**kwargs)

# PrimeLmOutput is a TypedDict (dict at runtime), HF outputs are dataclass-like objects
Expand Down
Loading
Loading