| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Signed-off-by: Yuchen Fan <functionhx@gmail.com>
There was a problem hiding this comment.
Hi @Functionhx,
Thank you for the fix! Overall this looks good to me. I would like to have a few changes before merge.
For the new test file, we now use this copyright (please refer to existing files):
# Copyright (c) DeepSpeed Team.
# SPDX-License-Identifier: Apache-2.0
# DeepSpeed TeamThe tests should assert that the wrapper and child own distinct pre/post backward hook classes and that both modules' applied_pre_backward_ref_cnt values return to zero after each backward. The current training loop can still pass if the counter lookup runs based on the existing hasattr behavior. (draft of the fix)
Sorry, something went wrong.
Signed-off-by: Yuchen Fan <functionhx@gmail.com>
|
@tohtana Thanks for the review! I've addressed the requested changes. Could you please take another look?🩷 |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you for the update! This looks good to me now.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Root cause
ZeRO-3 registers child modules before their parents. Wrappers such as PEFT's ModulesToSaveWrapper delegate unknown attributes to a wrapped child, so hasattr(wrapper, "pre_bwd_fn") and hasattr(wrapper, "post_bwd_fn") can incorrectly find the child's hook classes. DeepSpeed then skips creating wrapper-local classes even though those classes capture module-specific closures and counters. During forward, the delegated post-backward class can access the child before its ds_grads_remaining counter is initialized, raising AttributeError.
Check the module's own __dict__ for these internal attributes so each module receives hook classes and counters bound to itself.
User impact
This restores ZeRO-3 training with attribute-delegating wrappers, including PEFT LoRA configurations that use modules_to_save=["lm_head"].
Fixes #7615.
Testing