[feat] CPU mem: release and realloc gradient buffers when offload train engine - #719
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request optimizes memory utilization within the offload training engine by implementing a strategy to release and reallocate gradient buffers. By explicitly freeing gradient buffer GPU memory before offloading the model and then reallocating it upon resuming, the system avoids unnecessary transfer of gradient data to CPU memory, thereby reducing the overall memory footprint during the offload process. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a memory optimization by releasing and reallocating gradient buffers when the training engine is offloaded. The logic is sound and correctly placed within the sleep and wake_up methods. I have one suggestion to make the implementation more robust against potential errors during the buffer freeing process.
| self._grad_buffer_nbytes = [] | ||
| for buffer in self._get_grad_buffers(): | ||
| self._grad_buffer_nbytes.append(buffer.grad_data.untyped_storage().nbytes()) | ||
| buffer.grad_data.untyped_storage().resize_(0) |
There was a problem hiding this comment.
To improve robustness, it's better to first collect all buffer sizes and then resize them in a separate loop. This ensures that if an error occurs while getting a buffer's size, no buffers are resized, leaving the system in a consistent state. The current implementation could lead to a partially freed state if an error occurs mid-loop.
| self._grad_buffer_nbytes = [] | |
| for buffer in self._get_grad_buffers(): | |
| self._grad_buffer_nbytes.append(buffer.grad_data.untyped_storage().nbytes()) | |
| buffer.grad_data.untyped_storage().resize_(0) | |
| buffers = self._get_grad_buffers() | |
| self._grad_buffer_nbytes = [b.grad_data.untyped_storage().nbytes() for b in buffers] | |
| for buffer in buffers: | |
| buffer.grad_data.untyped_storage().resize_(0) |
| "Note: do not set `--ref-load` and `--keep-old-actor` if disable weights backuper." | ||
| ), | ||
| ) | ||
| parser.add_argument( |
There was a problem hiding this comment.
Do people think we need to keep this argument? Is there any senario that we need to keep cpu copy of gradient and distributed optimizer's parameter gather buffer? if no i will delete
There was a problem hiding this comment.
I personally do not have an example to need this flag in my mind
| "Note: do not set `--ref-load` and `--keep-old-actor` if disable weights backuper." | ||
| ), | ||
| ) | ||
| parser.add_argument( |
There was a problem hiding this comment.
I personally do not have an example to need this flag in my mind
ci-megatron-pr: #17
core changes
Megatron: radixark/Megatron-LM#17
torch memory saver: fzyzcjy/torch_memory_saver#72
result
Green is the newest
