Deepseek Engram optimization.#1147
Conversation
e6509b7 to
c7d57c3
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves DeepSeek Engram training performance by adding embedding-parallel communication options (all-to-all / all-reduce / offload), enabling precomputation/overlap of multi-head embedding work, and wiring new Engram-parallel arguments through initialization and example configs.
Changes:
- Add Engram embedding parallel method/size arguments and pass them into distributed initialization.
- Introduce an embedding-parallel
EngramMemoryimplementation and integrate it into Engram’s multi-head embedding + sharded checkpointing. - Add embedding precompute/caching (and attempt overlap across layers) plus new DeepSeek Engram Hydra configs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/functional_tests/train/deepseek/gold_values/tp2_pp2_ep2_engram.json | Reformat/update stored gold loss JSON. |
| flagscale/train/megatron/training/initialize.py | Plumb engram_embedding_parallel_size into model-parallel initialization. |
| flagscale/train/megatron/training/arguments_fs.py | Add Engram embedding parallel CLI args + validation/warnings. |
| flagscale/train/megatron/train_engram.py | Switch to parallel_state and add an extra TP token broadcast path. |
| flagscale/models/megatron/engram/multi_head_embedding.py | Add EngramMemory and route MultiHeadEmbedding through it; add sharded state dict. |
| flagscale/models/megatron/engram/engram_transformer_layer.py | Add embedding precompute hook and enable sharded state dict. |
| flagscale/models/megatron/engram/engram_model.py | Implement build_schedule_plan for Engram model. |
| flagscale/models/megatron/engram/engram_config.py | Add Engram embedding parallel config fields. |
| flagscale/models/megatron/engram/engram.py | Add embedding precompute/cache + sharded state dict plumbing; rename embedding member to memory. |
| examples/deepseek_v3/conf/train_engram.yaml | New top-level Hydra entry for Engram training run. |
| examples/deepseek_v3/conf/train/engram.yaml | New DeepSeek Engram training preset including embedding-parallel settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not (parallel_state.get_pipeline_model_parallel_world_size == 1 or parallel_state.is_pipeline_first_stage()): | ||
| if parallel_state.get_tensor_model_parallel_rank() == 0: | ||
| torch.distributed.broadcast(batch["tokens"], src=parallel_state.get_tensor_model_parallel_src_rank(), group=parallel_state.get_tensor_model_parallel_group()) | ||
| else: | ||
| tokens = torch.empty_like(batch["labels"]) | ||
| torch.distributed.broadcast(tokens, src=parallel_state.get_tensor_model_parallel_src_rank(), group=parallel_state.get_tensor_model_parallel_group()) | ||
| batch["tokens"] = tokens |
| from megatron.core.utils import get_attr_wrapped_model, StragglerDetector | ||
| from megatron.core.tokenizers.text.utils.build_tokenizer import build_tokenizer | ||
| from megatron.core import mpu | ||
| from megatron.core import parallel_state |
| (self.vocab_start_index, self.vocab_end_index) = ( | ||
| VocabUtility.vocab_range_from_global_vocab_size( | ||
| self.num_embeddings, get_pg_rank(self.embedding_parallel_group), get_pg_size(self.embedding_parallel_group) | ||
| ) |
| rank=get_pg_rank(self.tp_group), | ||
| world_size=get_pg_size(self.tp_group), |
| input_ids = input_ids.view(-1) | ||
| routing_map = input_ids // self.num_embeddings_per_partition | ||
| # [num_partitions], number of tokens assigned to each partition from the current rank's input. | ||
| num_tokens_per_partition = torch.histc(routing_map, bins=self.embedding_parallel_size, min=0, max=self.embedding_parallel_size) |
| warnings.warn(f"[rank0]: We do not recomend using allreduce for engram embedding, this is deprecated and will be removed in later version.", DeprecationWarning) | ||
| if self.args.engram_embedding_parallel_size is not None: |
| if parallel_state.get_tensor_model_parallel_rank() == 0: | ||
| torch.distributed.broadcast(batch["tokens"], src=parallel_state.get_tensor_model_parallel_src_rank(), group=parallel_state.get_tensor_model_parallel_group()) | ||
| else: | ||
| tokens = torch.empty_like(batch["labels"]) |
| self, | ||
| prefix: str = '', | ||
| sharded_offsets: Tuple[Tuple[int, int, int]] = (), | ||
| metadata: Optional[dict] = None,** kwargs, |
| assert not self.args.use_megatron_fsdp, "Megatron FSDP does not be supported yet, looking forward to later version." | ||
| assert not self.args.init_model_with_meta_device, "Init_model_with_meta_device does not be supported yet, looking forward to later version." |
| backend: torchrun | ||
| nnodes: 3 | ||
| nproc_per_node: 8 | ||
| hostfile: hostfile # Select an available hostfile. Like ip_1 slosts=8\nip_2 slost=8... |
9eded23 to
d0a886e
Compare
d060903 to
cb45ef4
Compare
1. All2All when compute embeddings. 2. Precompute multi_head_embedding.
…l and offload to be enabled simultaneously.
There was a problem hiding this comment.
Pull request overview
This PR improves DeepSeek Engram training by introducing an all-to-all–based embedding parallel path, adding embedding precompute overlap, and wiring in optional optimizer-state offloading and sharded-checkpoint support for Engram components.
Changes:
- Add Engram embedding parallel configuration (size/method) and optional embedding optimizer-state offloading flags.
- Implement an Engram all-to-all embedding path (
EngramMemory) plus embedding precompute/caching to overlap compute. - Update parameter-norm logic and distributed initialization to account for Engram embedding parallel groups; refresh functional test config/gold values and add example configs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/functional_tests/train/deepseek/gold_values/tp2_pp2_ep2_engram.json | Update expected loss gold values for the new Engram behavior/config. |
| tests/functional_tests/train/deepseek/conf/train/tp2_pp2_ep2_engram.yaml | Add Engram embedding parallel settings and adjust attention/MoE knobs for the functional test. |
| flagscale/train/megatron/training/utils.py | Track Engram embedding params separately and reduce their norm over an Engram-specific MP group. |
| flagscale/train/megatron/training/initialize.py | Plumb engram_embedding_parallel_size into model-parallel initialization. |
| flagscale/train/megatron/training/arguments_fs.py | Add Engram CLI args and post-validate logic for embedding parallel method/size and offload constraints. |
| flagscale/models/megatron/engram/multi_head_embedding.py | Implement EngramMemory all-to-all embedding path and route MultiHeadEmbedding between allreduce/alltoall methods; add sharded-state-dict handling. |
| flagscale/models/megatron/engram/engram_transformer_layer.py | Add embedding precompute hook and enable sharded-state-dict passthrough with a non-homogeneous layers flag. |
| flagscale/models/megatron/engram/engram_model.py | Add async hash computation wrapper and extend schedule-plan building to pass lazy hash inputs. |
| flagscale/models/megatron/engram/engram_config.py | Extend EngramConfig with embedding-parallel and offloading options. |
| flagscale/models/megatron/engram/engram.py | Add embedding precompute/caching via CUDA stream/event and implement sharded-state-dict composition. |
| examples/deepseek_v3/conf/train_engram.yaml | New runnable example wrapper config for Engram training. |
| examples/deepseek_v3/conf/train/engram.yaml | New full Engram training config (DeepSeek v3) including embedding-parallel settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| warnings.warn("Offloading embedding optimizer states will offload all embedding optimizer states to CPU, which may cause slowdown. " \ | ||
| "Please make sure this is what you want. This is typically used to save GPU memory when Engram embedding is large while accelerators are limited." \ | ||
| "If you do not want to offload all embedding optimizer states to CPU, please disable this and set the --optimizer-offload-fraction to a value less than 1 to offload part of the optimizer states to CPU." \ | ||
| "Of cource you can set the --optimizer-offload-fraction to offload other params meanwhile enable this to offload all embedding optimizer states to CPU.") |
There was a problem hiding this comment.
Typo in warning text: "Of cource" should be "Of course".
| "Of cource you can set the --optimizer-offload-fraction to offload other params meanwhile enable this to offload all embedding optimizer states to CPU.") | |
| "Of course you can set the --optimizer-offload-fraction to offload other params meanwhile enable this to offload all embedding optimizer states to CPU.") |
| self, | ||
| prefix: str = '', | ||
| sharded_offsets: Tuple[Tuple[int, int, int]] = (), | ||
| metadata: Optional[dict] = None,** kwargs, |
There was a problem hiding this comment.
EngramMemory.sharded_state_dict has invalid Python syntax in the parameter list: metadata: Optional[dict] = None,** kwargs, (space in ** kwargs and missing space after comma). This will raise a SyntaxError and prevent the module from importing. Change the signature to a valid **kwargs form (e.g., metadata: Optional[dict] = None, **kwargs).
| metadata: Optional[dict] = None,** kwargs, | |
| metadata: Optional[dict] = None, **kwargs, |
| # This is usefule when all layer are same, the TransformerBlock will be homogeneous, it generate sharded_state_dict will same keys for all layer and need all layers have the same structure. | ||
| # The layer has engram module does not fit this assumption. | ||
| # If the flag is set to True, the sharded_state_dict will use layer_number to generate different keys for different layer, which is same to models has dense layer leading and moe layer following. | ||
| # Actually, engram really causes the layers to be non-homogeneous. |
There was a problem hiding this comment.
sharded_state_dict() writes into metadata["non_homogeneous_layers"] without checking whether metadata is None. Since the method signature allows metadata: dict | None = None, this will raise a TypeError if callers omit metadata. Ensure metadata is initialized (e.g., set metadata = {} if metadata is None else metadata) before mutation.
| # Actually, engram really causes the layers to be non-homogeneous. | |
| # Actually, engram really causes the layers to be non-homogeneous. | |
| metadata = {} if metadata is None else metadata |
There was a problem hiding this comment.
Actually, metadata will never be None.
| def pre_compute_embedding(self, input_ids: torch.Tensor): | ||
| """ | ||
| Pre-compute the multi-head embedding for the given input IDs. | ||
| This can be called before the forward pass to warm up the embedding cache. | ||
| """ | ||
| assert input_ids is not None, "Input ids can not be None for EngramModel" | ||
| self.embedding_stream.synchronize() # Ensure previous computations on the stream are finished | ||
| with torch.cuda.stream(self.embedding_stream): | ||
| embedding_result = self.memory(input_ids).flatten(start_dim=-2) | ||
| embedding_event = torch.cuda.Event() | ||
| embedding_event.record(self.embedding_stream) | ||
| self.embedding_cache = (embedding_result, embedding_event) |
There was a problem hiding this comment.
pre_compute_embedding() unconditionally uses CUDA APIs (self.embedding_stream.synchronize(), torch.cuda.stream(...), torch.cuda.Event()) but self.embedding_stream is only created when torch.cuda.is_available(). If Engram is instantiated in a CPU-only run (or CUDA is disabled), this will raise an exception. Guard this method (and/or the call sites) so it either becomes a no-op on CPU or falls back to synchronous computation without CUDA streams/events.
| warnings.warn( | ||
| "[rank0]: If set the embedding_parallel_method to allreduce, " \ | ||
| "the embedding module will be the tensor_parallel.layers.VocabParallelEmbedding with tensor_parallel." \ | ||
| "So the embedding_parallel_size is useless and set to None." | ||
| ) |
There was a problem hiding this comment.
This warning message is labeled with "[rank0]" but is emitted on every rank (it isn't guarded by rank == 0). This can spam logs and is misleading. Either guard this warning with the same rank == 0 check, or remove the "[rank0]" prefix from the message.
| warnings.warn( | |
| "[rank0]: If set the embedding_parallel_method to allreduce, " \ | |
| "the embedding module will be the tensor_parallel.layers.VocabParallelEmbedding with tensor_parallel." \ | |
| "So the embedding_parallel_size is useless and set to None." | |
| ) | |
| if self.args.rank == 0: | |
| warnings.warn( | |
| "[rank0]: If set the embedding_parallel_method to allreduce, " \ | |
| "the embedding module will be the tensor_parallel.layers.VocabParallelEmbedding with tensor_parallel." \ | |
| "So the embedding_parallel_size is useless and set to None." | |
| ) |
|
|
||
|
|
||
| class EngramMemory(nn.Module): | ||
| """Embedding parallelized in the vocabulay dimension. |
There was a problem hiding this comment.
Typo in docstring: "vocabulay" should be "vocabulary".
| """Embedding parallelized in the vocabulay dimension. | |
| """Embedding parallelized in the vocabulary dimension. |
| # Engram let the layers be non-homogeneous, so we need to set the flag in metadata to let the sharded state dict logic know. | ||
| # This is usefule when all layer are same, the TransformerBlock will be homogeneous, it generate sharded_state_dict will same keys for all layer and need all layers have the same structure. | ||
| # The layer has engram module does not fit this assumption. | ||
| # If the flag is set to True, the sharded_state_dict will use layer_number to generate different keys for different layer, which is same to models has dense layer leading and moe layer following. | ||
| # Actually, engram really causes the layers to be non-homogeneous. |
There was a problem hiding this comment.
Typo in comment: "usefule" should be "useful".
| # Engram let the layers be non-homogeneous, so we need to set the flag in metadata to let the sharded state dict logic know. | |
| # This is usefule when all layer are same, the TransformerBlock will be homogeneous, it generate sharded_state_dict will same keys for all layer and need all layers have the same structure. | |
| # The layer has engram module does not fit this assumption. | |
| # If the flag is set to True, the sharded_state_dict will use layer_number to generate different keys for different layer, which is same to models has dense layer leading and moe layer following. | |
| # Actually, engram really causes the layers to be non-homogeneous. | |
| # Engram can make the layers non-homogeneous, so we need to set a flag in | |
| # metadata to let the sharded state dict logic know. | |
| # This is useful because when all layers are the same, TransformerBlock is | |
| # homogeneous and generates the same sharded_state_dict keys for all layers, | |
| # which requires all layers to have the same structure. | |
| # Layers with an Engram module do not fit this assumption. | |
| # If this flag is set to True, sharded_state_dict will use layer_number to | |
| # generate different keys for different layers, similar to models with dense | |
| # layers first and MoE layers following. |
ff8b531 to
1f6ad03
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.embedding_stream.synchronize() # Ensure previous computations on the stream are finished | ||
| with torch.cuda.stream(self.embedding_stream): | ||
| embedding_result = self.memory(input_ids).flatten(start_dim=-2) | ||
| embedding_event = torch.cuda.Event() | ||
| embedding_event.record(self.embedding_stream) | ||
| self.embedding_cache = (embedding_result, embedding_event) | ||
|
|
There was a problem hiding this comment.
pre_compute_embedding() unconditionally uses self.embedding_stream (synchronize() and torch.cuda.stream(...)), but self.embedding_stream is set only when torch.cuda.is_available(). If this method is called in a CPU-only environment (or before CUDA is initialized), it will raise an attribute error / CUDA error. Guard for self.embedding_stream is None (fallback to synchronous compute without streams/events) or assert CUDA availability with a clearer error message.
| self.embedding_stream.synchronize() # Ensure previous computations on the stream are finished | |
| with torch.cuda.stream(self.embedding_stream): | |
| embedding_result = self.memory(input_ids).flatten(start_dim=-2) | |
| embedding_event = torch.cuda.Event() | |
| embedding_event.record(self.embedding_stream) | |
| self.embedding_cache = (embedding_result, embedding_event) | |
| embedding_stream = getattr(self, "embedding_stream", None) | |
| if embedding_stream is not None and torch.cuda.is_available(): | |
| embedding_stream.synchronize() # Ensure previous computations on the stream are finished | |
| with torch.cuda.stream(embedding_stream): | |
| embedding_result = self.memory(input_ids).flatten(start_dim=-2) | |
| embedding_event = torch.cuda.Event() | |
| embedding_event.record(embedding_stream) | |
| else: | |
| embedding_result = self.memory(input_ids).flatten(start_dim=-2) | |
| embedding_event = None | |
| self.embedding_cache = (embedding_result, embedding_event) |
| engram_seed: 0 | ||
| engram_kernel_size: 4 | ||
| engram_hc_mult: 1 | ||
| engram_embedding_parallel_method: alltoall # alltoall, allreduce, offload |
There was a problem hiding this comment.
The inline comment lists offload as a valid value for engram_embedding_parallel_method, but the CLI/config choices added in this PR are only alltoall and allreduce (offloading is controlled by a separate boolean flag). Update the comment to avoid misleading users, and optionally document engram_offload_embedding_optimizer_states here instead.
| engram_embedding_parallel_method: alltoall # alltoall, allreduce, offload | |
| engram_embedding_parallel_method: alltoall # alltoall, allreduce | |
| # engram_offload_embedding_optimizer_states: true # Optional; controls optimizer-state offloading. |
| if self.args.engram_embedding_parallel_size is not None: | ||
| warnings.warn( | ||
| "[rank0]: If set the embedding_parallel_method to allreduce, " \ | ||
| "the embedding module will be the tensor_parallel.layers.VocabParallelEmbedding with tensor_parallel." \ | ||
| "So the embedding_parallel_size is useless and set to None." | ||
| ) | ||
| self.args.engram_embedding_parallel_size = None |
There was a problem hiding this comment.
engram_embedding_parallel_size is set to None only on rank 0 when engram_embedding_parallel_method == "allreduce". This makes argument values diverge across ranks, which can break any later logic that relies on consistent args (e.g., process-group initialization, padding logic, etc.). Keep the warning on rank 0 if desired, but apply the value normalization (engram_embedding_parallel_size = None) on all ranks (or broadcast a single normalized value).
| if self.args.engram_embedding_parallel_size is not None: | |
| warnings.warn( | |
| "[rank0]: If set the embedding_parallel_method to allreduce, " \ | |
| "the embedding module will be the tensor_parallel.layers.VocabParallelEmbedding with tensor_parallel." \ | |
| "So the embedding_parallel_size is useless and set to None." | |
| ) | |
| self.args.engram_embedding_parallel_size = None | |
| if self.args.engram_embedding_parallel_size is not None: | |
| if self.args.rank == 0: | |
| warnings.warn( | |
| "[rank0]: If set the embedding_parallel_method to allreduce, " \ | |
| "the embedding module will be the tensor_parallel.layers.VocabParallelEmbedding with tensor_parallel." \ | |
| "So the embedding_parallel_size is useless and set to None." | |
| ) | |
| self.args.engram_embedding_parallel_size = None |
| if extra_block_kwargs is None: | ||
| extra_block_kwargs = { | ||
| "engram_hash_input_ids": engram_hash_input_ids, | ||
| } |
There was a problem hiding this comment.
build_schedule_plan() only injects engram_hash_input_ids when extra_block_kwargs is None. If a caller passes a non-empty extra_block_kwargs, the schedule plan will be built without engram_hash_input_ids, but EngramTransformerBlock.forward() requires it. Merge into the provided dict (e.g., copy + setdefault) so engram_hash_input_ids is always present without clobbering user-supplied keys.
| if extra_block_kwargs is None: | |
| extra_block_kwargs = { | |
| "engram_hash_input_ids": engram_hash_input_ids, | |
| } | |
| extra_block_kwargs = dict(extra_block_kwargs or {}) | |
| extra_block_kwargs.setdefault("engram_hash_input_ids", engram_hash_input_ids) |
PR Category
[Train]
PR Types
[Improvements]
PR Description