CPU offloading fix: If Data and Transpose is None depend on super Torch tensor class for the shape - #2841
Conversation
for more information, see https://pre-commit.ci
Greptile SummaryThis PR fixes a crash during CPU offloading by replacing Confidence Score: 5/5Safe to merge — the fix is mechanically correct and all remaining findings are P2 style suggestions. The fallback to torch.Tensor.size(self) is the standard idiom for wrapper subclasses; shape metadata is always present in the C++ tensor created by _make_wrapper_subclass regardless of whether data storage is populated. All four tensor types are fixed consistently, every type in _quantization_list has a matching isinstance branch in the new test, and no blocking issues remain. No files require special attention; the P2 note about is_cuda/is_cpu in float8_tensor.py is a follow-up hardening suggestion, not a blocker. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[shape property called] --> B{_rowwise_data / _data\nis not None?}
B -- Yes --> C[Return data.shape\nor computed logical shape]
B -- No --> D{_transpose /\n_columnwise_data\nis not None?}
D -- Yes --> E[Return shape derived\nfrom transpose/columnwise data]
D -- No --> F[torch.Tensor.size self\nfallback to wrapper\nsubclass metadata]
F --> G[Returns shape set during\n_make_wrapper_subclass\nat construction time]
style F fill:#90EE90
style G fill:#90EE90
|
There was a problem hiding this comment.
Matches the patch I made.
@property
def shape(self):
"""Return the shape of the tensor. Define this to avoid expensive PyObject lookups."""
if self._data is not None:
return self._data.shape
if self._transpose is not None:
transpose_shape = self._transpose.shape
return torch.Size(tuple(transpose_shape[1:]) + (transpose_shape[0],))
return torch.Tensor.size(self)
I just did a quick 100-step Llama 8B test, I have loss parity as well:
[2026-04-06 16:36:16.442720] iteration 100/15258789 | consumed samples: 12800 | elapsed time per iteration (ms): 13432.5 | throughput per GPU (TFLOP/s/GPU): 1004.5 | learning rate: 4.915198E-07 | global batch size: 128 | lm loss: 1.261873E+00 | loss scale: 1.0 | grad norm: 5.902 | num zeros: 0 | number of skipped iterations: 0 | number of nan iterations: 0 |
|
/te-ci pytorch |
| transpose_shape = self._transpose.shape | ||
| return torch.Size(tuple(transpose_shape[1:]) + (transpose_shape[0],)) | ||
| raise RuntimeError("Both data and transpose are None") | ||
| return torch.Tensor.size(self) |
There was a problem hiding this comment.
Nit: This is correct, but it reads unpythonic to me. The following would be more standard:
| return torch.Tensor.size(self) | |
| return super(QuantizedTensor, self).size() |
| elif isinstance(x_test, Float8BlockwiseQTensor): | ||
| x_test._rowwise_data = None | ||
| x_test._columnwise_data = None | ||
|
|
There was a problem hiding this comment.
We don't want spurious test passes when we add new tensor types.
| else: | |
| raise NotImplementedError(f"{type(x_test).__name__} is not supported") | |
…ch tensor class for the shape (#2841) * fix Signed-off-by: Varun Thumbe <vthumbe@nvidia.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Signed-off-by: Varun Thumbe <vthumbe@nvidia.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ch tensor class for the shape (NVIDIA#2841) * fix Signed-off-by: Varun Thumbe <vthumbe@nvidia.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Signed-off-by: Varun Thumbe <vthumbe@nvidia.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
Please include a brief summary of the changes, relevant motivation and context.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: