-
Notifications
You must be signed in to change notification settings - Fork 659
Fix for async dcp checkpointing with Float8Tensors #2721
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
base: main
Are you sure you want to change the base?
Changes from all commits
641898c
5c34ffc
62d9300
5df7fb4
bbaef50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -152,6 +152,7 @@ def make_empty( | |||||||||||||||||||||||||||||||||||||||||||
| requires_grad=requires_grad, | ||||||||||||||||||||||||||||||||||||||||||||
| data_transpose=data_transpose, | ||||||||||||||||||||||||||||||||||||||||||||
| quantizer=self, | ||||||||||||||||||||||||||||||||||||||||||||
| device=device, | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| def calibrate(self, tensor: torch.Tensor) -> None: | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -379,6 +380,7 @@ def make_empty( | |||||||||||||||||||||||||||||||||||||||||||
| requires_grad=requires_grad, | ||||||||||||||||||||||||||||||||||||||||||||
| data_transpose=data_transpose, | ||||||||||||||||||||||||||||||||||||||||||||
| quantizer=self, | ||||||||||||||||||||||||||||||||||||||||||||
| device=device, | ||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| def calibrate(self, tensor: torch.Tensor) -> None: | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -953,6 +955,15 @@ def is_cuda(self): | |||||||||||||||||||||||||||||||||||||||||||
| return self._transpose.is_cuda | ||||||||||||||||||||||||||||||||||||||||||||
| raise RuntimeError("Both data and transpose are None") | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||||||||||||
| def is_cpu(self): | ||||||||||||||||||||||||||||||||||||||||||||
| """Return whether the tensor is on CPU.""" | ||||||||||||||||||||||||||||||||||||||||||||
| if self._data is not None: | ||||||||||||||||||||||||||||||||||||||||||||
| return self._data.is_cpu | ||||||||||||||||||||||||||||||||||||||||||||
| if self._transpose is not None: | ||||||||||||||||||||||||||||||||||||||||||||
| return self._transpose.is_cpu | ||||||||||||||||||||||||||||||||||||||||||||
| raise RuntimeError("Both data and transpose are None") | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| @classmethod | ||||||||||||||||||||||||||||||||||||||||||||
| def _make_in_reduce_ex( | ||||||||||||||||||||||||||||||||||||||||||||
| cls, | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -977,7 +988,14 @@ def _make_in_reduce_ex( | |||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| def __reduce_ex__(self, protocol: int) -> tuple: | ||||||||||||||||||||||||||||||||||||||||||||
| """Custom pickling to remove references to FP8 metadata objects""" | ||||||||||||||||||||||||||||||||||||||||||||
| """Custom pickling to remove references to FP8 metadata objects | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| CPU Float8Tensors are serialized as dequantized plain tensors | ||||||||||||||||||||||||||||||||||||||||||||
| for compatibility with torch.load(weights_only=True), which is | ||||||||||||||||||||||||||||||||||||||||||||
| used by DCP async save staging. | ||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||
| if self.is_cpu: | ||||||||||||||||||||||||||||||||||||||||||||
| return self.dequantize(dtype=self.dtype).__reduce_ex__(protocol) | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+997
to
+998
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The A safe guard would be: def __reduce_ex__(self, protocol: int) -> tuple:
if self._data is not None and self._data.is_cpu:
return self.dequantize(dtype=self.dtype).__reduce_ex__(protocol)
return (
Float8Tensor._make_in_reduce_ex,
(self._data, self._fp8_dtype, self._scale_inv, self.dtype, self.shape),
)This covers only the case where
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wait, didn't i just change this from the other comment starting with "CPU-only transpose path not handled in reduce_ex"?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right — I suggested changing to The The fix needs to handle all three cases:
Here's a clean solution using try-except:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||
| Float8Tensor._make_in_reduce_ex, | ||||||||||||||||||||||||||||||||||||||||||||
| (self._data, self._fp8_dtype, self._scale_inv, self.dtype, self.shape), | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,7 @@ | |
|
|
||
| from ...quantized_tensor import QuantizedTensorStorage, Quantizer | ||
|
|
||
| from ...constants import TE_DType as torch_to_transformer_engine_dtype | ||
| from ...constants import TE_DType as torch_to_transformer_engine_dtype, TE_DType_To_Torch | ||
|
|
||
| from ...utils import is_non_tn_fp8_gemm_supported, _empty_tensor | ||
|
|
||
|
|
@@ -35,6 +35,13 @@ def forward( | |
| if tensor._data is not None: | ||
| if tensor._data.numel() == 0: | ||
| return torch.empty_like(tensor._data, dtype=dtype) | ||
| if tensor._data.is_cpu: | ||
| # CPU fallback: reinterpret uint8 as FP8, cast to target dtype, scale | ||
| fp8_torch_dtype = TE_DType_To_Torch[tensor._fp8_dtype] | ||
| return ( | ||
| tensor._data.view(fp8_torch_dtype).float() | ||
| * tensor._scale_inv.to(tensor._data.device) | ||
| ).to(dtype) | ||
| # Cast from FP8 | ||
| return tex.dequantize(tensor, te_dtype) | ||
|
|
||
|
|
@@ -132,6 +139,7 @@ def get_metadata(self) -> Dict[str, Any]: | |
| "fp8_dtype": self._fp8_dtype, | ||
| "data_transpose": self._transpose, | ||
| "quantizer": self._quantizer, | ||
| "device": self.device, | ||
| "fake_dtype": self._dtype, | ||
|
Comment on lines
141
to
143
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Adding Before this PR, A safe guard: "device": self._data.device if self._data is not None
else (self._transpose.device if self._transpose is not None else None), |
||
| } | ||
|
|
||
|
|
||
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.
AttributeErrorwhen_quantizerisNonetensor._quantizercan beNoneforFloat8Tensorobjects deserialized via the GPU path (_make_in_reduce_ex), which does not pass aquantizerargument. If a second async DCP save is attempted after a load/save round-trip,new_emptywill be dispatched on the deserialized tensor, causingAttributeError: 'NoneType' object has no attribute 'make_empty'.A guard is needed before calling
make_empty: