Skip to content

Conversation

@eternalNight
Copy link
Contributor

DeepSpeedZeroOptimizer provides a rich, evolving list of keyword arguments. It is tedious and error-prone to list all of them in its subclasses. As an example, the recent introduction of zenflow_config in the middle of that list has caused unit test failures (e.g. https://github.com/deepspeedai/DeepSpeed/actions/runs/18560070656/job/52906645682?pr=7633)

Convert the keyword argument list in DeepSpeedZeroOptimizer subclasses to **kwargs for the consistency of configurable items and their default values. Passing an unknown parameter to such subclasses will now raise an error on their call to DeepSpeedZeroOptimizer.init() instead of their own init(). It still ensures that typo in such parameters fail early.

DeepSpeedZeroOptimizer provides a rich, evolving list of keyword
arguments. It is tedious and error-prone to list all of them in its
subclasses. As an example, the recent introduction of zenflow_config in
the middle of that list has caused unit test failures (e.g.
https://github.com/deepspeedai/DeepSpeed/actions/runs/18560070656/job/52906645682?pr=7633)

Convert the keyword argument list in DeepSpeedZeroOptimizer subclasses
to **kwargs for the consistency of configurable items and their default
values. Passing an unknown parameter to such subclasses will now raise
an error on their call to DeepSpeedZeroOptimizer.__init__() instead of
their own __init__(). It still ensures that typo in such parameters fail
early.

Signed-off-by: Junjie Mao <[email protected]>
@eternalNight
Copy link
Contributor Author

eternalNight commented Oct 17, 2025

It looks the CI still used the old code base (see https://github.com/deepspeedai/DeepSpeed/actions/runs/18582440763/job/52979937206?pr=7634#step:5:473).

@sfc-gh-truwase Any idea what's going wrong here?

@tohtana
Copy link
Contributor

tohtana commented Oct 20, 2025

Hi @eternalNight,
We have been using the test code on the master branch since we migrated to Modal. There are both technical and security reasons. (But I thought only the tests are taken from master and the PR branch is used for deepspeed).
For this one, I confirmed it pass the test on my env. So let me approve and merge it.

@tohtana tohtana merged commit 2734a6a into deepspeedai:master Oct 20, 2025
15 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants