-
Notifications
You must be signed in to change notification settings - Fork 9k
[v1] refactor registry plugin structure and params #10641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -93,9 +93,12 @@ def __init__( | |||||||||||||||||||
| dist_name = self.args.dist_config.name if self.args.dist_config is not None else None | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if dist_name == "deepspeed": | ||||||||||||||||||||
| from ..plugins.trainer_plugins.distributed.hub import DistributedPlugin | ||||||||||||||||||||
| if self.args.cp_size > 1: | ||||||||||||||||||||
| raise ValueError("Context parallelism currently requires `dist_config.name: fsdp2`.") | ||||||||||||||||||||
|
|
||||||||||||||||||||
| self._deepspeed_engine = DistributedPlugin("deepspeed")( | ||||||||||||||||||||
| from ..plugins.trainer_plugins.distributed.interface import DistributedPlugin | ||||||||||||||||||||
|
|
||||||||||||||||||||
| self._deepspeed_engine = DistributedPlugin("deepspeed").shard_model( | ||||||||||||||||||||
| self.model, | ||||||||||||||||||||
| self.args.dist_config, | ||||||||||||||||||||
| num_micro_batch=self.train_batch_generator.num_micro_batch, | ||||||||||||||||||||
|
|
@@ -138,7 +141,7 @@ def __init__( | |||||||||||||||||||
| self.state.global_step = self.global_step | ||||||||||||||||||||
| self.state.epoch = self._resume_epoch | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if self.args.dist_config is not None and self.args.dist_config.get("cp_size", 1) > 1: | ||||||||||||||||||||
| if self.args.cp_size > 1: | ||||||||||||||||||||
| # qwen3.5 is not supported because of the different attention implementation, which will be supported in the future. | ||||||||||||||||||||
| if model.config.model_type == "qwen3_5": | ||||||||||||||||||||
| raise RuntimeError( | ||||||||||||||||||||
|
|
@@ -151,7 +154,7 @@ def __init__( | |||||||||||||||||||
| "Sequence parallelism requires flash attention. Please set `flash_attn: flash_attention_2`." | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| SequenceParallelModelPlugin(self.args.dist_config.get("cp_mode", "ulysses"))(model, self.args.dist_config) | ||||||||||||||||||||
| SequenceParallelModelPlugin(self.args.cp_mode)(model, self.args.cp_size) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def _create_batch_generator(self) -> None: | ||||||||||||||||||||
| if ( | ||||||||||||||||||||
|
|
@@ -182,9 +185,9 @@ def _shard_model(self) -> None: | |||||||||||||||||||
| device_ids = None if self.device.type == "cpu" else [self.device.index] | ||||||||||||||||||||
| self.model = DDP(self.model, device_ids=device_ids) | ||||||||||||||||||||
| else: | ||||||||||||||||||||
| from ..plugins.trainer_plugins.distributed.hub import DistributedPlugin | ||||||||||||||||||||
| from ..plugins.trainer_plugins.distributed.interface import DistributedPlugin | ||||||||||||||||||||
|
|
||||||||||||||||||||
| self.model = DistributedPlugin(self.args.dist_config.name)( | ||||||||||||||||||||
| self.model = DistributedPlugin(self.args.dist_config.name).shard_model( | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If
Suggested change
|
||||||||||||||||||||
| self.model, | ||||||||||||||||||||
| self.args.dist_config, | ||||||||||||||||||||
| bf16=self.args.bf16, | ||||||||||||||||||||
|
|
@@ -255,7 +258,7 @@ def fit(self) -> None: | |||||||||||||||||||
| step_valid_tokens = DistributedInterface().all_reduce(step_valid_tokens, op=ReduceOp.SUM) | ||||||||||||||||||||
| num_micro = len(micro_batches) | ||||||||||||||||||||
| for i, micro_batch in enumerate(micro_batches): | ||||||||||||||||||||
| if self.args.dist_config and self.args.dist_config.get("cp_size", 1) > 1: | ||||||||||||||||||||
| if self.args.cp_size > 1: | ||||||||||||||||||||
| from ..plugins.model_plugins.parallelization.sequence_parallel import ( | ||||||||||||||||||||
| SequenceParallelLossPlugin, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
@@ -281,7 +284,7 @@ def fit(self) -> None: | |||||||||||||||||||
| else: | ||||||||||||||||||||
| grad_norm = torch.nn.utils.clip_grad_norm_(self.model.parameters(), self.args.max_grad_norm).item() | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if self.args.dist_config and self.args.dist_config.get("cp_size", 1) > 1: | ||||||||||||||||||||
| if self.args.cp_size > 1: | ||||||||||||||||||||
| grad_norm = grad_norm**2 | ||||||||||||||||||||
| grad_norm = DistributedInterface().all_reduce(grad_norm, op=ReduceOp.SUM, dim=Dim.CP) | ||||||||||||||||||||
| grad_norm = grad_norm**0.5 | ||||||||||||||||||||
|
|
@@ -342,7 +345,7 @@ def fit(self) -> None: | |||||||||||||||||||
| def save_model(self) -> None: | ||||||||||||||||||||
| """Save the model.""" | ||||||||||||||||||||
| if self.args.dist_config is not None and self.args.dist_config.name in ("deepspeed", "fsdp2"): | ||||||||||||||||||||
| from ..plugins.trainer_plugins.distributed.hub import DistributedPlugin | ||||||||||||||||||||
| from ..plugins.trainer_plugins.distributed.interface import DistributedPlugin | ||||||||||||||||||||
|
|
||||||||||||||||||||
| DistributedPlugin(self.args.dist_config.name).save_model( | ||||||||||||||||||||
|
Comment on lines
347
to
350
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If
Suggested change
|
||||||||||||||||||||
| self.model, self.args.output_dir, self.renderer.processor | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
self.args.dist_configis a standard dictionary,self.args.dist_config.namewill raise anAttributeError. Usinggetattr(self.args.dist_config, 'name', None)is safer and more robust.