Skip to content

Store FP8 checkpointing data in CPU - #351

Merged
ksivaman merged 1 commit into
NVIDIA:mainfrom
ksivaman:store_fp8_extra_state_in_cpu
Aug 2, 2023
Merged

Store FP8 checkpointing data in CPU#351
ksivaman merged 1 commit into
NVIDIA:mainfrom
ksivaman:store_fp8_extra_state_in_cpu

Conversation

@ksivaman

@ksivamanksivaman commented Aug 2, 2023

Copy link
Copy Markdown
Member

See comment for the need for this.

Note: This is backwards compatible with current checkpoints including all parallelism modes (DP/SP/TP/PP).

Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
@ksivaman

Copy link
Copy Markdown
MemberAuthor

/te-ci

Comment on lines +90 to +96
buffer = {}

# Map all tensors to CPU.
for k, v in _global_fp8_buffer.items():
buffer[k] = [tensor.cpu() for tensor in v]

return buffer

@timmoon10timmoon10Aug 2, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like the right place for checkpoint-specific logic. How about we change

state["global_fp8_buffer"] =get_global_fp8_buffer()

state["global_fp8_buffer"] = {
key: [tensor.cpu() fortensorintensor_list]
forkey, tensor_listinget_global_fp8_buffer().items()
}

@ksivamanksivamanAug 2, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_global_fp8_buffer is a checkpoint specific function. Its only use is to return the current global buffer for saving the checkpoint.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this implementation of get_global_fp8_buffer mirrors the implementation of set_global_fp8_buffer:

defset_global_fp8_buffer(buffer: Dict[str, List[torch.Tensor]]) ->None:
"""Sets global fp8 buffer."""
global_global_fp8_buffer
# Map all tensors back to GPU.
fork, vinbuffer.items():
buffer[k] = [tensor.cuda() fortensorinv]
_global_fp8_buffer=buffer

That said, it's unintuitive. The function names and documentation don't make me think of checkpointing.

@timmoon10
timmoon10 self-requested a review August 2, 2023 22:50
@ksivaman
ksivaman merged commit 85928d0 into NVIDIA:mainAug 2, 2023
mingxu1067 pushed a commit to mingxu1067/TransformerEngine that referenced this pull request Aug 3, 2023
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Ming Huang <mingh@nvidia.com>
timmoon10 added a commit that referenced this pull request Aug 3, 2023
* Cast Flax collections to FrozenDict as WAR to adapt Flax 0.7.1.
Signed-off-by: Ming Huang <mingh@nvidia.com>
* Adding min version of flax to requirements.txt in examples
Signed-off-by: Ming Huang <mingh@nvidia.com>
* Fix praxis tests and rename compare_frozen_dict to compare_dict
Signed-off-by: Ming Huang <mingh@nvidia.com>
* [Paddle] Refactor FP8 state (#350)
Refactor fp8 state
Signed-off-by: Tian Zheng (Engrg-Hardware 1) <tizheng@nvidia.com>
Signed-off-by: Ming Huang <mingh@nvidia.com>
* Store FP8 checkpointing data in CPU (#351)
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Ming Huang <mingh@nvidia.com>
* Make test_layer be able to run on both Flax >=0.7.1 and <=0.7.0
Signed-off-by: Ming Huang <mingh@nvidia.com>
* Update Flax version
Signed-off-by: Tim Moon <tmoon@nvidia.com>
---------
Signed-off-by: Ming Huang <mingh@nvidia.com>
Signed-off-by: Tian Zheng (Engrg-Hardware 1) <tizheng@nvidia.com>
Signed-off-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Signed-off-by: Tim Moon <tmoon@nvidia.com>
Co-authored-by: Tian Zheng <tizheng@nvidia.com>
Co-authored-by: Kirthi Shankar Sivamani <ksivamani@nvidia.com>
Co-authored-by: Tim Moon <tmoon@nvidia.com>
Sign up for freeto 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

@ksivaman@timmoon10