Skip to content

emo integration - #841

Open
AkshitaB wants to merge 63 commits into
mainfrom
akshitab/emo-integration
Open

AkshitaB wants to merge 63 commits into
mainfrom
akshitab/emo-integration

Conversation

@AkshitaB

Copy link
Copy Markdown
Contributor

No description provided.

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

elif isinstance(self.trainer.train_module, OLMoDDPTrainModule):
optimizers = [self.trainer.train_module.optim]
scheduler = self.trainer.train_module.scheduler

P1 Badge Rebuild the PP schedule when changing the batch size

When an OLMoDDPTrainModule also has pp_config, this newly enabled callback changes only the data loader's global batch size and optimizer LR. OLMoDDPTrainModule.on_attach() computes the pipeline schedule's num_microbatches once from the original batch size, so later warmup events keep the old microbatch count and change the per-microbatch tensor size instead of preserving rank_microbatch_size; an increase can therefore exceed the configured memory bound and OOM, while some decreases fail the pipeline divisibility check. Update the pipeline schedule's microbatch count when applying the event, or reject this callback for pipeline-enabled OLMoDDP.


beaker_image = opts.beaker_image
if beaker_image == OLMoCoreBeakerImage.stable:
for preset in presets:

P2 Badge Preserve an explicitly selected stable image

When a preset is combined with an explicit --beaker-image whose value is the stable image, this equality check treats that explicit choice as though the flag were omitted and replaces it with the preset image. This violates the documented --beaker-image > preset > default precedence and can launch on a materially different CUDA/GPU image; use a None/sentinel parser default so omission can be distinguished from explicitly requesting the stable image.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

return x_grad, None, None, None, None

P1 Badge Return one gradient per async all-to-all input

When a training path using all_to_all_async() runs backward, autograd expects four gradient entries because forward() accepts four inputs after ctx, but this returns five. The new ep_sync_1d path exercises this operation twice, so its backward pass fails with an incorrect-number-of-gradients error before updating the model; remove the extra trailing None.


else:
# no last stage output
final_lm_output = None

P1 Badge Make pipeline evaluation return output on every rank

With pipeline parallelism enabled, ranks that do not own the final stage return None here, but every rank runs EvaluatorCallback.perform_eval(), which immediately asserts that eval_batch() returned an LMOutputWithLoss before participating in metric collectives. Consequently, any configured in-loop or offline evaluation crashes on the non-final pipeline ranks; the final-stage result must be communicated or the evaluator must explicitly coordinate non-final ranks.


pre_setup = _chain(*[preset.pre_setup for preset in presets], opts.pre_setup)
post_setup = _chain(*[preset.post_setup for preset in presets], opts.post_setup)

P2 Badge Let explicit setup flags replace preset steps

When a caller combines a preset with --pre-setup or --post-setup, these lines execute both commands even though the new CLI help and LaunchPreset documentation state that explicit setup flags override the preset. For example, --preset olmo-ddp --post-setup ... cannot replace the preset's CUDA-extension build, so users cannot bypass an incompatible or failing preset step without dropping all other preset defaults.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

else:
raise OLMoConfigurationError(
f"pipeline schedule {schedule_name.value!r} is not supported by this train module. "
f"Only the custom schedules are wired up: "
f"{PipelineScheduleType.custom_interleaved_1F1B.value!r} and "
f"{PipelineScheduleType.custom_1F1B_V.value!r}. Standard PyTorch schedules "
"(1F1B, Interleaved1F1B, GPipe, ...) are not currently supported here."
)

P1 Badge Preserve support for standard pipeline schedules

This rejects every pre-existing pipeline schedule, including the default PipelineParallelConfig.schedule (interleaved_1F1B). Consequently existing TransformerPipelineTrainModule configurations now fail during schedule construction; for example, src/examples/moe/train.py still explicitly selects PipelineScheduleType.interleaved_1F1B. Keep the former PyTorch schedule path for standard schedule names and restrict the new custom driver to the custom_* variants.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review


P1 Badge Handle standard schedules before iterating pipeline outputs

When OLMoDDPTrainModuleConfig uses TransformerPipelineParallelConfig without overriding its default Interleaved1F1B schedule, PipelineSchedule.step() returns an (output, losses) tuple rather than the nested per-stage/per-microbatch list assumed here. Non-final ranks will try to iterate None, while final ranks can fail the LMOutputWithLoss assertion, so an otherwise default PP configuration cannot complete its first training step; either reject/force a custom schedule for this module or handle the standard schedule's return contract.



P1 Badge Remove the unsupported pipeline-evaluation kwarg

Whenever an OLMoDDPTrainModule with pipeline parallelism performs an in-loop evaluation, this argument is forwarded through run_pipeline_eval() into the custom schedule's _split_inputs(). That splitter only accepts SUPPORTED_MODEL_KWARGS, which does not contain batch_num_tokens_for_loss, so every such evaluation raises ValueError: Unsupported kwargs for pipeline splitting before any forward pass.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

if self.eval_only:
sd_to_load = self._get_model_state_dict_for_eval_load(metadata)
dist_cp.state_dict_loader.load(
sd_to_load,

P1 Badge Refresh FP8 caches after eval-only checkpoint loads

When rowwise_fp8 is enabled, eval-only construction prequantizes the randomly initialized expert weights in OLMoDDPTrainModule._parallelize_model(), but this load branch only overwrites the model parameters. Unlike the training load path, it never refreshes the FP8 weight stores or rowwise prequantization caches, so checkpoint evaluation can run with stale random expert weights and produce invalid metrics. Refresh each model part's rowwise FP8 cache after loading the checkpoint parameters.


checkpoint_key = self._resolve_model_checkpoint_key(name, checkpoint_keys)
if checkpoint_key is None:
continue

P1 Badge Reject incomplete eval-only checkpoint loads

If an eval checkpoint lacks any parameter expected by the configured model—for example after a config mismatch or parameter rename—this silently skips that parameter while loading all other weights. Since eval-only model construction initializes parameters before loading, the missing parameter remains randomly initialized and evaluation can complete with corrupted results; collect missing names and fail the load instead of accepting any nonempty partial state.


# Per-head learnable attention-sink logits (GPT-OSS). See :meth:`sdpa`.
self.sinks: Optional[nn.Parameter] = (
nn.Parameter(torch.empty(n_heads, dtype=dtype, device=init_device))
if attention_sinks
else None

P1 Badge Shard attention-sink logits under tensor parallelism

With attention_sinks=True and tensor parallelism greater than one, Q/K/V contain only the local head shard, while this parameter remains replicated with all n_heads; apply_tp() explicitly shards ssmax_scale but never sinks. The sink backend then tries to concatenate logits shaped with local heads and sink logits shaped with global heads, causing a dimension mismatch on the first forward pass. Distribute sinks with Shard(0) alongside the other per-head parameter.


elif sinks is not None:
# The sink path applies softmax manually (see below), so it needs an explicit causal
# mask rather than relying on SDPA's ``is_causal``.
attn_mask = self._get_sliding_window_mask(
seq_len_q=q.shape[1],
seq_len_kv=k.shape[1],
device=q.device,
window_size=(-1, -1),
)

P2 Badge Build the sink mask after Ulysses redistribution

For full attention with sinks and Ulysses context parallelism, this mask is built from the pre-redistribution local sequence lengths (T/CP), but the following all_to_all_*_cp2hp calls expand Q/K to the global sequence length T. The manual sink path consequently adds a (T/CP, T/CP) mask to (T, T) attention weights and fails at runtime. Construct the causal mask after the CP-to-head redistribution, or explicitly reject this combination.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review


P2 Badge Restore optimizer state even when checkpoint saves fail

If the distributed checkpoint write raises because of an I/O or upload failure, control never reaches the subsequent optim.load_state_dict(...). This matters because optim.state_dict() deliberately replaces some live EP-DP optimizer-state shards with empty tensors while preparing the checkpoint, so a caller that catches the error or retries the save is left with a corrupted in-memory optimizer. Restore the state in a finally block or make state-dict creation non-destructive.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

original_get_event_by_index = streams._get_event_by_index
original_get_stream_by_index = streams._get_stream_by_index

P1 Badge Guard the stream lookup patch on supported older Torch

When running with PyTorch 2.6, which pyproject.toml still explicitly supports via torch>=2.6.0, prepare_cli_environment() now calls this patch unconditionally, but older torch._dynamo.variables.streams versions do not expose the 2.11-era _get_event_by_index/_get_stream_by_index APIs (and may also lack the device-agnostic torch.Event, torch.Stream, and torch.accelerator APIs used below). Consequently normal training and launch scripts can fail during environment setup before doing any work. Return without patching unless all required stream and accelerator APIs are present, or raise the minimum Torch version accordingly.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Base automatically changed from akshitab/moe-v2-core to main September 14, 2026 18:29
YashasSamaga and others added 28 commits September 15, 2026 17:28
Adds inference-time expert restriction to Olmo3MoeRouter for the EMo modularity
metric: a non-persistent allowed_experts buffer with set/clear methods, applied
to the router scores immediately before the ordinary token-level top-k. Masking
scores rather than logits matches the training-time EMo document-pool operator,
which selects on masked scores but weights with the unmasked ones.

The mask is evaluation state rather than model configuration, so it stays out of
the state dict and a single checkpoint can be scored repeatedly under different
subsets. Also adds get_moe_routers() to collect routers by layer index, skipping
dense layers.

Tests cover all-experts parity as an exact no-op, exclusion of masked experts,
restoration after clearing, rejection of malformed and undersized masks, and
non-persistence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
EMo derives per-token `segment_ids` from token IDs and EOS positions,
but only the first pipeline stage receives token IDs — later stages
receive hidden states. Both the model and the pipeline schedule rejected
the combination outright.

The batch is already available on every pipeline rank, and keyword
arguments (unlike positional ones) reach every stage, so segment IDs
need no transport. They are derived once per rank and passed through as
a side input.

- `emo_block_indices` / `emo_eos_token_id` on `Transformer` replace the
per-forward block scan. Both resolve lazily, after the pipeline split,
so a stage sees only the blocks it owns.
- `_prepare_inputs` accepts caller-supplied `segment_ids`, deriving them
from token IDs only as the non-pipelined fallback. It has to be an
explicit pop — unrecognized kwargs are otherwise dropped silently.
- Both EMo+PP guards removed (`_prepare_inputs` and `apply_pp`). The
adjacent context-parallelism guard stays.

- `segment_ids`, `doc_lens`, and `max_doc_lens` are now split into
microbatches. `max_doc_lens` is a per-instance Python list, so it is
sliced rather than passed through the tensor branch. Intra-document
masking was already broken under PP for this reason, independent of EMo.
- Every batch-leading value splits on one set of shared, equal
boundaries instead of independent `tensor_split` calls.
- Uneven microbatches are rejected. Stages size their P2P buffers from a
single floor-divided microbatch shape (`prepare_step` →
`example_p2p_tensor`), so larger leading chunks would overrun the
receiver — a buffer shape mismatch, not just inconsistent metadata. The
check sits after the active microbatch count is selected, so training,
evaluation, and reduced dry runs each validate against the count they
actually run with.
- Batch size is inferred from any batch-leading kwarg, not just
`labels`, so a stage receiving `segment_ids` without `labels` still
works.

`run_pipeline_eval` padding now covers batch-leading lists and tuples,
which a list-valued `max_doc_lens` would otherwise be left short by. The
reduced-microbatch dry-run path already handled these.

- Two-batch overlap still rejects EMo separately: segment IDs are not
split between lanes.
- Context parallelism is unchanged and still rejected.
- Cross-*rank* disagreement on the EOS token ID is unvalidated; catching
it needs a collective. Disagreement across the model parts a single rank
holds is caught.

`pytest src/test/` passes apart from pre-existing failures (GCS
credential tests, and the reshard suite, both confirmed identical on a
clean tree). New coverage:

- Model-side: EMo block/EOS discovery, derivation vs. caller-supplied
precedence, the PP guard, shape validation, and routing segment IDs to
blocks given a hidden-state input.
- Splitter: alignment with `input_ids`, packed-document metadata,
uneven-batch rejection, batch-size inference on later stages.
- Evaluation padding for lists, tuples, and tensors.
- A multi-GPU execution test asserting each stage receives the correct
microbatch's segment IDs, including a rank that gets no token IDs at
all. **This one has not been run** — it requires GPUs.

A second commit temporarily adds this base branch to the workflow
triggers, mirroring the existing `akshitab/moe-v2-core` entries. Revert
before merging the base branch into `main`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@AkshitaB
AkshitaB force-pushed the akshitab/emo-integration branch from 44adac8 to 82fb4a7 Compare September 16, 2026 00:31

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82fb4a70f4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

self.disable_mxfp8_expert_anchor_grads()

dp_group = dense_process_group if dense_process_group is not None else dp_mesh.get_group()
load_balancing_group = dp_mesh.get_group()

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 Include CP ranks in global load-balancing counts

When context parallelism is enabled, each CP rank owns only a sequence shard, but this group reduces counts only across the dp_mesh. The loss is subsequently gradient-reduced across the combined DP+CP group, so each CP shard weights its local router scores using a different partial count vector; averaging those products is not equivalent to computing the loss from global assignment counts and scores. Use the supplied combined dense_process_group (or otherwise reduce across both DP and CP) so global_load_balancing=True trains against the intended global distribution.

Useful? React with 👍 / 👎.

Comment on lines +131 to +132
if already_interpolated:
paths.append(f"{base_dir}{path}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject incompatible tokenizers for the hard-coded mix

For Dolma3p5_14t, this branch ignores the tokenizer argument and always returns files tokenized with allenai/dolma2-tokenizer. Selecting the new mix with any other tokenizer configuration can therefore either feed out-of-range token IDs to the embedding or silently train on IDs with the wrong vocabulary semantics. Validate that the requested tokenizer is Dolma2-compatible instead of accepting every tokenizer.

Useful? React with 👍 / 👎.

Comment on lines +178 to +179
if getattr(hf_config, "model_type", None) == "olmo3moe":
roundtrip_state = convert_state_from_hf(hf_config, hf_state_dict, model_type="olmo3moe")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid reconstructing the entire MoE state for validation

Every olmo3moe export now reconstructs a complete OLMo state while both the original state and converted HF state are still live. The reverse converter stacks and concatenates the per-expert tensors, so this is another full-model allocation rather than a lightweight view; for the tens-of-billions-parameter models supported by this change it adds hundreds of gigabytes of peak host memory and can make an otherwise viable conversion OOM before writing anything. Validate mappings incrementally or make this expensive round-trip check opt-in.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants