-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix wrong documentation of ignore_unused_parameters
#4418
Conversation
Hi @tjruwase, the original commit encountered some unexpected unit test errors even if it only modified the docstring. Could you kindly check if there's anything wrong? Thanks. |
Hi @UniverseFly - there is a known CI issues we are working on. There doesn't appear to be anything wrong with your PR, we will get it merged shortly, thanks! |
) Co-authored-by: Olatunji Ruwase <[email protected]> Co-authored-by: Logan Adams <[email protected]>
i think we still see the inconsistent documentation on this.. |
Hi @keunwoochoi - could you link the inconsistent docs? We'd certainly like to fix them if there is anything inconsistent |
hi @loadams, yes absolutely. on the doc about the json (https://www.deepspeed.ai/docs/config-json/), the "Description" column says |
@keunwoochoi - the docs inconsistency should be fixed here: #4949 Link to the code where the default is set: https://github.com/microsoft/DeepSpeed/blob/13d84b4912492d40b68f4da7bc570e47b54bce2a/deepspeed/runtime/zero/config.py#L242C4-L242C42 |
@loadams thanks! |
The documentation says:
Hence the inconsistency.