Skip to content

Fix hardcoded instances to fp16 in optimizer creation log messages to the correct dtype. #2743

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.

Sign up for GitHub

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

Merged
loadams merged 13 commits into from
Jan 31, 2023
Merged

Fix hardcoded instances to fp16 in optimizer creation log messages to the correct dtype. #2743

Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6b8347a
Remove hardcoded instances to fp16 in log messages.
loadams Jan 24, 2023
a8556e0
Update docutils version to fix errors with sphinx
loadams Jan 24, 2023
293e70e
Revert "Update docutils version to fix errors with sphinx"
loadams Jan 24, 2023
faf565b
Merge branch 'master' into loadams/fix-fp16-bf16-logging-issue
tjruwase Jan 24, 2023
7631665
Fix for YAPF
loadams Jan 24, 2023
71402dc
Merge branch 'master' into loadams/fix-fp16-bf16-logging-issue
loadams Jan 24, 2023
503170c
Add model_dtype to print the correct format
loadams Jan 30, 2023
64a61ae
Missed once instance
loadams Jan 30, 2023
e2b2871
Merge conflicts
loadams Jan 30, 2023
3b23ce8
Merge conflicts messed up pre-commit
loadams Jan 30, 2023
fa7c0f2
Merge branch 'master' into loadams/fix-fp16-bf16-logging-issue
loadams Jan 30, 2023
2b985e0
Merge branch 'master' into loadams/fix-fp16-bf16-logging-issue
loadams Jan 31, 2023
b34edfe
Respond to PR feedback
loadams Jan 31, 2023
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
18 changes: 10 additions & 8 deletions deepspeed/runtime/engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1433,14 +1433,16 @@ def _configure_fp16_optimizer(self, optimizer):
initial_dynamic_scale = self.initial_dynamic_scale()
dynamic_loss_args = self.dynamic_loss_scale_args()
clip_grad = self.gradient_clipping()
model_dtype, grad_accum_dtype = self.get_data_types()
if APEX_INSTALLED:
fused_opts = (apex.optimizers.FusedAdam, FusedAdam)
else:
fused_opts = FusedAdam
if isinstance(optimizer, fused_opts) \
or self.optimizer_name() in [ONEBIT_ADAM_OPTIMIZER, ZERO_ONE_ADAM_OPTIMIZER]:
if self.dynamic_loss_scale():
log_dist("Creating fp16 optimizer with dynamic loss scale", ranks=[0])
log_dist(f'Creating {model_dtype} optimizer with dynamic loss scale',

@SaulLu SaulLu Jan 31, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For this message, we initialize only the FP16_Optimizer class afterwards, it is perhaps safer not to change the message in this case, no?

@loadams loadams Jan 31, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I was somewhat hoping to future proof it so we don't end up in the same scenario as last time, but these do make sense to leave as fp16. I'll make that change.

@loadams loadams Jan 31, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed now.

@SaulLu SaulLu Jan 31, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you! Everything looks good for me

ranks=[0])
timers = self.timers if self.wall_clock_breakdown() else None
optimizer = FP16_Optimizer(
optimizer,
Expand All @@ -1456,10 +1458,8 @@ def _configure_fp16_optimizer(self, optimizer):
)
else:
log_dist(
"Creating fp16 optimizer with static loss scale: {}".format(
self.loss_scale()),
ranks=[0],
)
f'Creating {model_dtype} optimizer with static loss scale: {self.loss_scale()}',
ranks=[0])
optimizer = FP16_Optimizer(
optimizer,
deepspeed=self,
Expand All @@ -1470,7 +1470,7 @@ def _configure_fp16_optimizer(self, optimizer):
has_moe_layers=self.has_moe_layers,
)
else:
log_dist("Creating fp16 unfused optimizer with dynamic loss scale",
log_dist(f'Creating {model_dtype} unfused optimizer with dynamic loss scale',

@SaulLu SaulLu Jan 31, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would have the same question for this message since the only optimizer that is initialized after is optimizer = FP16_UnfusedOptimizer

@loadams loadams Jan 31, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed now, along with one additional fix of this.

ranks=[0])
optimizer = FP16_UnfusedOptimizer(
optimizer,
Expand Down Expand Up @@ -1507,6 +1507,7 @@ def _configure_bf16_optimizer(self, optimizer):

def _configure_zero_optimizer(self, optimizer):
zero_stage = self.zero_optimization_stage()
model_dtype, grad_accum_dtype = self.get_data_types()
assert self.communication_data_type in (torch.float16, torch.bfloat16), "ZeRO supports only 'communication_data_type': ['fp16', 'bfp16']"
timers = self.timers if self.wall_clock_breakdown() else None

Expand All @@ -1524,7 +1525,8 @@ def _configure_zero_optimizer(self, optimizer):
round_robin_gradients = self.zero_round_robin_gradients()
assert not isinstance(optimizer, DummyOptim), "zero stage {} requires an optimizer".format(zero_stage)

log_dist('Creating ZeRO stage {} optimizer'.format(zero_stage), ranks=[0])
log_dist(f'Creating {model_dtype} ZeRO stage {zero_stage} optimizer',
ranks=[0])
# Overlap and contiguous grads are meaningless in stage 1 and are ignored
if zero_stage == ZeroStageEnum.optimizer_states:
overlap_comm = False
Expand Down Expand Up @@ -1588,7 +1590,7 @@ def _configure_zero_optimizer(self, optimizer):
offload_param_config=self.zero_offload_param(),
mpu=self.mpu)
else:
log_dist('Creating fp16 ZeRO stage {} optimizer'.format(zero_stage),
log_dist(f'Creating {model_dtype} ZeRO stage {zero_stage} optimizer',
ranks=[0])
from deepspeed.runtime.zero.stage3 import DeepSpeedZeroOptimizer_Stage3
optimizer = DeepSpeedZeroOptimizer_Stage3(
Expand Down