Skip to content
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

[Bug Fixed] scale=INF when casting a tensor to scaling FP32/BF16 tensors #131

Merged
merged 4 commits into from
Nov 28, 2023

Conversation

wkcn
Copy link
Contributor

@wkcn wkcn commented Nov 18, 2023

Description
The following script triggers the bug.

import torch
from msamp.common.dtype import Dtypes
torch.tensor([1.0 / 512], device='cuda').cast(Dtypes.kfloat32)

It will return ScalingTensor(tensor([nan], device='cuda:0'), meta=ScalingMeta(qtype=QType(name='kFloat32', value=2), scale=inf, scale_inv=0, amax=0.00195312, window_size=1).

The reason is that the scaling factor is stored as a FP32 scalar and it is equal to the maximum representation value / the maximum absolute value.

When the maximum absolute value is less than 1.0, the scaling factor will be larger than the maximum representation value of FP32, so the scaling factor is INF and its inverse scale_inv is 0.

To address the issue, the scaling factor is set to 1 when the data type is FP32 or BF16. After fixing the bug, it outputs

ScalingTensor(tensor([0.0020], device='cuda:0'), meta=ScalingMeta(qtype=QType(name='kFloat32', value=2), scale=1, scale_inv=1, amax=0.00195312, window_size=1)

@tocean
Copy link
Contributor

tocean commented Nov 27, 2023

Looks like the scaling factor will be always 1 for bf16 and float32. Do we still need to compute amax in TypeCast::cast_to_fp16 for bf16 and fp32? Compute amax may be time consuming.

@wkcn
Copy link
Contributor Author

wkcn commented Nov 27, 2023

Looks like the scaling factor will be always 1 for bf16 and float32. Do we still compute amax in TypeCast::cast_to_fp16 for bf16 and fp32? Compute amax may be time consuming.

Good catch! I will remove the computation of amax for BF16 and FP32.

@wkcn
Copy link
Contributor Author

wkcn commented Nov 27, 2023

@tocean I have tried to remove the amax calculation in ScalingBF16/FP32, but it affects the other calculation logic. I would prefer to optimize this in other late PRs.

@wkcn wkcn enabled auto-merge (squash) November 28, 2023 00:34
@tocean
Copy link
Contributor

tocean commented Nov 28, 2023

@tocean I have tried to remove the amax calculation in ScalingBF16/FP32, but it affects the other calculation logic. I would prefer to optimize this in other late PRs.

Sure.

Copy link
Contributor

@tocean tocean left a comment

Choose a reason for hiding this comment

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

LGTM

@wkcn wkcn merged commit c69de04 into Azure:main Nov 28, 2023
9 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.

3 participants