[PyTorch][torch.compile] Make get_attention_backend traceable without graph breaks#3189
[PyTorch][torch.compile] Make get_attention_backend traceable without graph breaks#3189pggPL wants to merge 12 commits into
Conversation
…reaks - Read NVTE_* env vars via os.environ.get instead of os.getenv so dynamo installs guards on the values (os.getenv reads are not guarded and would bake stale backend selections into compiled graphs). - Wrap tex.get_fused_attn_backend in a torch.compiler.assume_constant_result helper so the pybind call does not graph-break. - Mark get_device_compute_capability/get_cudnn_version with assume_constant_result for the same reason. - Use a no-op logger when compiling (logging.Logger methods graph-break) and skip debug-log blocks that call int()/str() on pybind enums. - Add test in tests/pytorch/test_torch_compile.py checking fullgraph=True tracing and recompilation on NVTE_* env var changes. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Greptile SummaryThis PR makes
Confidence Score: 5/5Safe to merge. The backend-selection logic is correct in all paths; the FusedAttnBackend IntEnum mirrors the C++ enum exactly with an import-time assert as a safety net; os.environ.get guards and assume_constant_result are applied consistently; and backward compatibility is preserved through cast() normalisation in fused_attn_fwd/bwd. All changes are in selection/dispatch logic with no tensor computation affected. The only findings are quality observations: warning/error logs are silently dropped during compilation (users lose fallback notices under torch.compile), a minor type annotation inaccuracy in ne, and three per-version env-var keys left out of the test pre-set. None of these affect correctness or introduce data-path regressions. transformer_engine/pytorch/attention/dot_product_attention/utils.py deserves a second look regarding the _NoOpLogger swallowing warning/error log levels. tests/pytorch/test_torch_compile.py is missing NVTE_FLASH_ATTN_V2/V3/V4 pre-sets which limits dynamo guard coverage for those keys. Important Files Changed
Reviews (8): Last reviewed commit: "[PyTorch] Address review: add info()/err..." | Re-trigger Greptile |
Comparing the pybind enum returned through assume_constant_result against module-level enum values generates guards dynamo cannot evaluate (crash when the comparison is true, i.e. when cuDNN rejects the config). The wrapper now returns a plain int, comparisons use precomputed int values, and the enum for callers is reconstructed by a second assume_constant_result helper that is never compared during tracing. Also document that os.environ.get (vs os.getenv) is intentional, and drop the guard_scalar specialization of numeric args: symbolic scalars (automatic dynamic) now graph break at the probe instead of forcing a full recompile per seqlen value; the test covers that path without fullgraph and checks the selection stays correct. A second test monkeypatches tex.get_fused_attn_backend to verify the baked result is trace-time-only and actually drives selection. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
The pybind NVTE_Fused_Attn_Backend enum is not traceable by torch.compile: its C-implemented __eq__ cannot be traced, and a pybind enum instance baked through assume_constant_result produces guards dynamo cannot evaluate when compared against module-level enum values. FusedAttnBackend is now a plain python IntEnum generated at import time from tex.NVTE_Fused_Attn_Backend.__members__ (values always in sync with the C enum), and all remaining direct uses of the pybind enum on the python side are replaced with it. Name lookup (FusedAttnBackend["FP8"]) behaves the same as with the previous dict, and the backend value never crosses into a pybind call, so no boundary conversion is needed. This removes the previous int-based workaround in get_attention_backend (_fused_attn_backend_from_int and the precomputed int table): the assume_constant_result wrapper now simply returns the IntEnum and comparisons are traceable directly. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Mirror how constants.DType migrated off the pybind enum: explicit IntEnum members pinned to the C values with an import-time sync assert, an __eq__ override comparing by integer value against NVTE_Fused_Attn_Backend (with matching __ne__/__hash__) so mixed comparisons stay equivalent regardless of the pybind11 version, and a cast() classmethod. fused_attn_fwd/bwd normalize their fused_attention_backend argument through cast(), so external callers still passing the pybind enum keep working. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
test_get_attention_backend_traceable_fp8 compiles the selection with fullgraph=True for AttentionParams(fp8=True) with a DelayedScaling(fp8_dpa) recipe, covering the FP8-only branch (run_config env reads, recipe filters, get_fp8_te_dtype) and checks that flipping NVTE_UnfusedDPA_Emulate_FP8 recompiles and keeps matching eager. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
The is_compiling() guards around the available/selected-backend debug logs protected int() on the pybind enum, which crashed dynamo during tracing. With FusedAttnBackend now a python IntEnum, int() on it and str() on the flash-attn PkgVersion both trace cleanly (verified under fullgraph=True), so the logging blocks return to their upstream shape. The probe wrapper also reuses FusedAttnBackend.cast() and a shorter docstring. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Merge the three get_attention_backend tests into one covering: fullgraph tracing with the probe consulted at trace time only, env var flips (F16 and FP8), attention-param changes, and a forced No_Backend result driving the selection. The bitmask output now also encodes the fused sub-backend, which previously went unchecked. The probe wrapper takes layout/bias/mask/softmax as string keys and resolves the pybind enums internally, so every argument is a literal or a python enum - required for assume_constant_result(specialize_args=True) (pytorch#189042) to derive value guards once available. Scalars must stay concrete until then: the test pins specialize_int/float=True, because a symbolic scalar currently graph breaks at the probe and dynamo's resume then corrupts the returned fused backend (binds the wrapper function object instead of its result; surfaced by the sub-backend bits, minimal repro exists). Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
The probe-argument and static-scalar comments referenced assume_constant_result(specialize_args=True), which is not part of any released PyTorch; describe the current behavior only. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Drop the call-counting monkeypatch and its assertions; compiled-vs-eager output equality is what matters and already fails on stale selections. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
…tion_backend_traceable # Conflicts: # tests/pytorch/test_torch_compile.py
|
/te-ci pytorch |
…tion_backend_traceable Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com> # Conflicts: # transformer_engine/pytorch/attention/dot_product_attention/utils.py
|
/te-ci pytorch |
shino16: _NoOpLogger does not subclass logging.Logger, so any traced logger.info()/logger.error() call under torch.compile would raise AttributeError. Add the two no-op methods for completeness. Signed-off-by: Pawel Gadzinski <pgadzinski@nvidia.com>
Description
This PR is part of an effort to enable
torch.compilesupport for attention: it makesget_attention_backendtraceable.It works on current PyTorch as-is — there is no need to wait for the two related PRs I opened in PyTorch. Those only improve the remaining rough edge: pytorch/pytorch#189042 (bigger and much more important; among other things it handles symbolic scalar arguments, which with stock dynamo must stay non-symbolic in the backend-selection probe — with static scalars, the common case, everything in this PR already works) and pytorch/pytorch#189027 (small, and possibly not needed anymore since this PR reads env vars via
os.environ.get, which stock dynamo already guards).get_attention_backendcurrently graph-breaks undertorch.compile, forcing backend selection to run as an eager island. This PR makes it fully traceable withfullgraph=True, with dynamo guards on theNVTE_*environment variables so that changing an env var triggers recompilation instead of silently reusing a stale backend selection.Type of change
Changes
Read
NVTE_*env vars inget_attention_backendviaos.environ.getinstead ofos.getenv: dynamo installs value guards onos.environaccesses, whileos.getenvreads are unguarded (the stdlib-global source is skipped) and would bake a stale backend selection into the compiled graph.Wrap the
tex.get_fused_attn_backendpybind call in atorch.compiler.assume_constant_resulthelper — the result depends only on the attention configuration, not on tensor values.Mark
get_device_compute_capabilityandget_cudnn_versionwithassume_constant_result(pybind/CUDA property calls are not traceable, and their results are constant for a process).Use a no-op logger when compiling (
logging.Loggermethods graph-break in dynamo) and skip the debug-log blocks that evaluateint()/str()on pybind enum values during tracing. Backend-selection debug logs are unchanged in eager mode.Add
test_get_attention_backend_traceabletotests/pytorch/test_torch_compile.py: compiles a function callingget_attention_backendwithfullgraph=True(any graph break fails the test) and flipsNVTE_FUSED_ATTN/NVTE_UNFUSED_ATTN/NVTE_FLASH_ATTNto verify the guards trigger recompilation and the result keeps matching eager.FusedAttnBackend(inpytorch/cpp_extensions/fused_attn.py) is now a pythonIntEnumgenerated at import time fromtex.NVTE_Fused_Attn_Backend.__members__, replacing the previous str->pybind-enum dict, and all direct python-side uses of the pybind enum are replaced with it. Rationale: the pybind enum is not traceable by dynamo (C__eq__), and a pybind enum baked throughassume_constant_resultproduces guards dynamo cannot evaluate when compared against module-level enum values (hard crash when cuDNN rejects a config). The transition follows the pattern used forconstants.DType: explicit members pinned to the C values with an import-time sync assert, an__eq__override so mixed comparisons withtex.NVTE_Fused_Attn_Backendstay equivalent regardless of the pybind11 version, and acast()classmethod;fused_attn_fwd/bwdnormalize their backend argument throughcast(), so callers still passing the pybind enum keep working. Name lookupFusedAttnBackend["FP8"]andint(...)behave as before. The remaining (minor) breaking surface isisinstancechecks against the pybind enum and dict-API such as.items()/iteration; no such uses exist inside TE and none were found in Megatron-LM/NeMo.Note: dynamo only guards
os.environkeys that exist at trace time; reads of absent keys are not guarded yet (upstream PyTorch limitation).Checklist: