DeepSpeed
f76ab880 - Drop the documented grad_hooks ZeRO option, which does not exist (#8242)

Commit
11 days ago
Drop the documented grad_hooks ZeRO option, which does not exist (#8242) ## Problem `config-json.md` documents `grad_hooks` as a ZeRO option with a default of `True`: > **grad_hooks**: [boolean] > For use with ZeRO stage 1, enable backward hooks to reduce gradients during the backward pass or wait until the end of the backward pass. Default `True` `DeepSpeedZeroConfig` has no such field, and `DeepSpeedConfigModel` sets `extra="forbid"`, so anyone who follows the docs gets a hard failure out of `deepspeed.initialize`: ```python DeepSpeedZeroConfig(**{"stage": 1, "grad_hooks": False}) pydantic_core._pydantic_core.ValidationError: 1 validation error for DeepSpeedZeroConfig grad_hooks Extra inputs are not permitted [type=extra_forbidden, input_value=False, input_type=bool] ``` The option was never wired up rather than removed later, so there is nothing to restore. `cfa63f5da` ("ZeRO stage 1 refresh", #1042, thanks @ebarkhordar for pinning it down) added the doc entry and `DeepSpeedEngine.zero_grad_hooks()` in the same commit and never added the field to `deepspeed/runtime/zero/config.py` or to the ZeRO constants module. It is absent from both in every release back to v0.3.0. `zero_grad_hooks()` reads `self._config.zero_config.grad_hooks`, which can only raise `AttributeError`, and nothing in the package or the tests calls it. ## Fix Remove the doc entry and the dead accessor. If the intent is that the option should exist, that is a feature rather than a fix and I would rather leave it to you than invent a semantic for it. ## Test No test. An earlier revision of this PR added a guard to `tests/unit/runtime/zero/test_zero_config.py` that checked every ZeRO key `config-json.md` documents is one `DeepSpeedZeroConfig` accepts. @tohtana asked for it to be dropped as too fragile for CI, which is fair, so it is gone and this is now a two file deletion. What was checked instead, on CPU: - The `ValidationError` above reproduces on `master` and the doc entry is what invites it. - An `ast` sweep over every `.py` in the tree finds `zero_grad_hooks` exactly once, its own definition at `deepspeed/runtime/engine.py:1290`, and `.grad_hooks` exactly once, on the line inside it. Removing it cannot break a caller, since any call raises `AttributeError` today. - `python -m pytest tests/unit/runtime/zero/test_zero_config.py` gives 6 passed before and after, unchanged, and that file is now untouched by this PR. - `yapf --style .style.yapf` and `flake8 --config .flake8` are clean on `deepspeed/runtime/engine.py`, and clean on the unmodified tree as a control. ## How it was found Cross-checking every `<i>**key**</i>` in `config-json.md` against a `grep` for that literal in `deepspeed/`. Of 126 documented keys only two came back unreferenced: `Compression`, which is a section name, and this one. For scope rather than as a request: the drift runs the other way too, with 28 fields `DeepSpeedZeroConfig` declares having no entry in that doc section. That direction misleads rather than crashes, so it is left alone here. Signed-off-by: Vineeth Sai <vineethsai4444@gmail.com> Co-authored-by: Masahiro Tanaka <81312776+tohtana@users.noreply.github.com>
Author
Parents
Loading