Skip to content

Add the getter and setter of skip_fp8_weight_update_tensor#3015

Open
xrennvidia wants to merge 4 commits into
NVIDIA:mainfrom
xrennvidia:xren/fix_skip_fp8_weight_update
Open

Add the getter and setter of skip_fp8_weight_update_tensor#3015
xrennvidia wants to merge 4 commits into
NVIDIA:mainfrom
xrennvidia:xren/fix_skip_fp8_weight_update

Conversation

@xrennvidia
Copy link
Copy Markdown
Collaborator

@xrennvidia xrennvidia commented May 20, 2026

Description

The getter and setter of skip_fp8_weight_update_tensor were deleted in @pggPL 's PR2759, but MCore local Cuda Graph implementation still needs it (like here), so create this PR to recover it back.

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Change A
  • Change B

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Signed-off-by: Xiaowei Ren <xren@nvidia.com>
@xrennvidia xrennvidia requested a review from ksivaman as a code owner May 20, 2026 09:48
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR restores the get_skip_fp8_weight_update_tensor and set_skip_fp8_weight_update_tensor class methods on FP8GlobalStateManager that were removed in PR #2759, and updates the two inline call sites in graph.py to use the new setter instead of directly mutating quantization_state.

  • set_skip_fp8_weight_update_tensor lazily allocates the CUDA float32 tensor if it does not yet exist and fills it with the provided bool value; this matches the logic that was previously inlined in graph.py.
  • get_skip_fp8_weight_update_tensor returns the tensor or None if it has never been set, giving external consumers (e.g., MCore CUDA graph code) a stable, public-ish API without direct attribute access on the internal dataclass.

Confidence Score: 5/5

Safe to merge. The change is a small, targeted restoration of two accessor methods with no logic changes to the underlying tensor lifecycle.

Both call sites in graph.py are updated to use the new setter, and the setter itself is an exact extraction of the logic that was previously inlined — no behavioral difference. The getter simply exposes what was already a public dataclass field. The only loose end is three existing callers in module files that still access the field directly, but those predate this PR and are not regressed by it.

No files require special attention. module/linear.py, module/layernorm_mlp.py, and module/layernorm_linear.py still access the internal field directly, but that is pre-existing and out of scope for this PR.

Important Files Changed

Filename Overview
transformer_engine/pytorch/quantization.py Adds set_skip_fp8_weight_update_tensor and get_skip_fp8_weight_update_tensor class methods to FP8GlobalStateManager; encapsulates tensor creation and retrieval behind a clean API.
transformer_engine/pytorch/graph.py Replaces two direct accesses to quantization_state.skip_fp8_weight_update_tensor with calls to the new setter; no logic change.

Sequence Diagram

sequenceDiagram
    participant MCore as MCore CudaGraph
    participant FP8GSM as FP8GlobalStateManager
    participant QState as FP8GlobalState (quantization_state)

    MCore->>FP8GSM: set_skip_fp8_weight_update_tensor(True/False)
    alt tensor is None
        FP8GSM->>QState: create skip_fp8_weight_update_tensor (CUDA float32)
    end
    FP8GSM->>QState: fill_(skip)

    MCore->>FP8GSM: get_skip_fp8_weight_update_tensor()
    FP8GSM->>QState: read skip_fp8_weight_update_tensor
    QState-->>MCore: Optional[torch.Tensor]

    Note over FP8GSM,QState: graph.py also calls set_skip_fp8_weight_update_tensor during graphed callable setup and forward pass
Loading

Reviews (3): Last reviewed commit: "Merge branch 'main' into xren/fix_skip_f..." | Re-trigger Greptile

Comment thread transformer_engine/pytorch/quantization.py Outdated
return type fix

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Xiaowei Ren <103958965+xrennvidia@users.noreply.github.com>
@ptrendx ptrendx requested a review from pggPL May 21, 2026 00:39
Copy link
Copy Markdown
Member

@ptrendx ptrendx left a comment

Choose a reason for hiding this comment

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

I believe there could be a reason why Pawel removed those functions from this object and we may need to change MCore instead in order to have this be compatible with torch.compile. Setting 'request changes' status for now until @pggPL reviews it.

@github-actions github-actions Bot added the community-contribution PRs from external contributor outside the core maintainers, representing community-driven work. label May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution PRs from external contributor outside the core maintainers, representing community-driven work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants