-
Notifications
You must be signed in to change notification settings - Fork 203
feat. Add more optimizer choice #431
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
Conversation
/gemini review |
PR #366 should put after this PR |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…into optimizer_addddd
Clear for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, waiting for other reviewer's comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, only one small issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request expands optimizer support in the codebase, allowing users to select between Adam, SGD, and a new AnyPrecision AdamW optimizer (adam_bf16) for improved mixed-precision training.
- Adds support for
sgd
andadam_bf16
optimizer types across CLI validation, FSDP, and Megatron engines - Implements a custom
AnyPrecisionAdamW
optimizer with flexible precision and optional Kahan summation - Refactors optimizer creation from base class to engine-specific implementations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
docs/cli_reference.md |
Updates documentation to reflect new optimizer choices and parameter applicability |
areal/utils/fsdp/optimizer.py |
Adds new AnyPrecisionAdamW optimizer implementation with precision control |
areal/experimental/megatron_engine.py |
Updates Megatron engine to support AdamW and SGD optimizers |
areal/engine/ppo/actor.py |
Minor formatting changes (whitespace additions) |
areal/engine/fsdp_engine.py |
Implements optimizer creation with support for all three optimizer types |
areal/engine/base_hf_engine.py |
Refactors optimizer creation to abstract method pattern |
areal/api/cli_args.py |
Updates CLI argument validation and adds version check logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
/gemini review |
/gemini review |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Done. Review again please @garrett4wade . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request expands optimizer support in the codebase, allowing users to select between Adam, SGD, and a new AnyPrecision AdamW optimizer (adam_bf16). It also introduces a custom implementation of AnyPrecisionAdamW for improved mixed-precision training and updates documentation and CLI argument validation accordingly.
Optimizer Support Enhancements:
sgd
andadam_bf16
optimizer types inOptimizerConfig
, and updated CLI argument validation to reflect these new options.areal/engine/base_hf_engine.py
) to allow selection between AdamW, SGD, and the new AnyPrecisionAdamW optimizer, invoking the appropriate optimizer based on user configuration.areal/experimental/megatron_engine.py
) to allow both AdamW and SGD optimizers, passing the selected type to the optimizer configuration.New Optimizer Implementation:
AnyPrecisionAdamW
optimizer inareal/utils/fsdp/optimizer.py
, supporting flexible precision (bfloat16/float32) and optional Kahan summation for improved numerical stability during mixed-precision training.AdamW_bf16/SGD optimizer maybe useful to avoid OOM, but less stability
Note
AnyPrecisionAdamW should not be changed as it is from other repo's implementation.
PR #366 should put after this PR
Optimizer Support