Repository navigation
Conversation
…formBase Extends PlatformBase with two optional hooks -- torch_profiler_activity() and torch_profiler_content_name() -- so a hardware backend can plug its own torch.profiler.ProfilerActivity into get_torch_profiler() the same way CUDA's "cuda" content already works, instead of needing its own monkey-patch (as Cambricon MLU's plugin currently does for this exact gap). Both default to None, so CUDA/ROCm/NPU/existing plugins are unaffected. Companion PlatformXPU implementation: verl-hardware-plugin feature/xpu-torch-profiler (not yet merged). Test: PYTHONPATH=. pytest tests/plugin/test_platform_abstraction.py -v (15 passed, including the new default-value check) Co-authored-by: Claude
…validation TorchProfilerToolConfig.__post_init__ hardcoded its allowed-contents whitelist, so a plugin's torch_profiler_content_name() (e.g. "xpu") passed config-time validation for the hook resolution but was then rejected here before get_torch_profiler() ever saw it -- an AssertionError, not the hook silently no-op'ing. Found via a real GRPO/FSDP2/vLLM training run on Intel XPU with profiler.tool_config.torch.contents=[xpu,cpu]; confirmed working end-to-end afterward (real trace produced during the actual training step, not just the isolated get_torch_profiler() call this whitelist bypassed). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Sep 23, 2026
kahlun
added a commit
to kahlun/verl-hardware-plugin-fork
that referenced
this pull request
Sep 24, 2026
Adds torch_profile_xpu.py, mirroring the existing MLU pattern (torch_profile_mlu.py): monkeypatches TorchProfilerToolConfig to accept "xpu" in profiler.tool_config.torch.contents, and wraps get_torch_profiler to collect torch.profiler.ProfilerActivity.XPU when requested. Verified on real B60 hardware: a torch.profiler run with contents=[xpu,cpu] produces a populated Chrome trace containing genuine Intel Level Zero driver activity (xpu_driver/xpu_runtime categories, zeDeviceGetRootDevice etc.). Replaces the PlatformXPU.torch_profiler_activity()/ torch_profiler_content_name() approach from #6, which depended on new PlatformBase hooks proposed in verl-project/verl#7988. Per the maintainer's ruling on #26, the default should be plugin-side monkeypatch, not new verl-core abstraction, and this feature has no dispatch point that requires a core change -- so no core PR is needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
kahlun
added a commit
to kahlun/verl-hardware-plugin-fork
that referenced
this pull request
Sep 24, 2026
Adds torch_profile_xpu.py, mirroring the existing MLU pattern (torch_profile_mlu.py): monkeypatches TorchProfilerToolConfig to accept "xpu" in profiler.tool_config.torch.contents, and wraps get_torch_profiler to collect torch.profiler.ProfilerActivity.XPU when requested. Verified on real B60 hardware: a torch.profiler run with contents=[xpu,cpu] produces a populated Chrome trace containing genuine Intel Level Zero driver activity (xpu_driver/xpu_runtime categories, zeDeviceGetRootDevice etc.). Replaces the PlatformXPU.torch_profiler_activity()/ torch_profiler_content_name() approach from #6, which depended on new PlatformBase hooks proposed in verl-project/verl#7988. Per the maintainer's ruling on #26, the default should be plugin-side monkeypatch, not new verl-core abstraction, and this feature has no dispatch point that requires a core change -- so no core PR is needed.
1 task done
…gin hook Require torch_profiler_content_name() to also be non-None before adding a platform's torch.profiler activity, matching the hook contract that both values must be present. Previously an activity-only hook (no content name) was silently included whenever `contents` was empty. Document the two new PlatformBase hooks in the platform README's Interface Summary table, and add unit tests covering: activity added for empty and matching `contents`, excluded for non-matching `contents`, and excluded when either half of the activity/content-name pair is missing.
kahlun
force-pushed
the
feature/torch-profiler-plugin-hook-clean
branch
from
September 24, 2026 07:24
39d0bdf to
f816689
Compare
Copilot's re-review on this PR found the same paired-hook gap in a second spot: TorchProfilerToolConfig.__post_init__ whitelisted a platform's torch_profiler_content_name() whenever it was non-None, without also checking torch_profiler_activity(). A config naming that keyword (e.g. contents=["xpu"]) therefore passed validation even when the platform had no activity to pair with it -- get_torch_profiler() would then silently drop the content instead of erroring at config time. Require both hooks before whitelisting the keyword, matching the same paired-hook contract already enforced in get_torch_profiler(). Add regression tests mirroring TestPluginProfilerActivityHook.
physics31415926
pushed a commit
to verl-project/verl-hardware-plugin
that referenced
this pull request
Sep 29, 2026
## The bug `torch_profile_mlu.py` monkey-patches two verl-core functions at import time, unconditionally, regardless of which hardware platform is actually active: - `TorchProfilerToolConfig.__post_init__` - `verl.utils.profiler.torch_profile.get_torch_profiler` Both patches save a reference to the original (`_original_post_init`, `_original_get_torch_profiler`) but never call it — they reimplement the logic from scratch instead. That reimplementation had drifted from the real thing, and breaks every non-MLU platform that goes through these same shared entry points: 1. **Whitelist bug** — the patched `__post_init__` hardcodes an allowed-values list for `contents` (`mlu, cuda, cpu, memory, shapes, stack`). Any other platform's own contribution (e.g. a plugin adding `"xpu"` via its own `torch_profiler_content_name()`) gets rejected with a confusing `AssertionError`, because the real `__post_init__` that would know about it is never invoked. 2. **Signature drift bug** — the hand-copied `_mlu_get_torch_profiler(contents, save_path, role, save_file_prefix, rank, schedule)` doesn't match the real `get_torch_profiler`, which has since grown new params (`profile_step`, `name_mini_batch_window`). Any caller on any platform using those newer params hits a `TypeError`, because MLU's patch fully replaced the real function. Confirmed on real Intel B60 hardware: a GRPO/FSDP2/vLLM training step with `profiler.tool_config.torch.contents=[xpu, cpu]` hit bug 2's `TypeError`. Bug 1's `AssertionError` also fired in that run, but only because a local, not-yet-upstreamed XPU `torch_profiler_content_name()` contribution had made `"xpu"` an allowed value in the first place — verified against stock `verl-project/verl` (at `9097cc4`) and this plugin as they exist today, neither has `"xpu"` in `__support_contents` and there's no content-name hook contributing it, so `contents=[xpu, ...]` raises the same `AssertionError` with or without MLU's patch installed. Bug 1 is a real structural issue (below), but on stock code today it isn't independently attributable to MLU — only bug 2 is. ## Concrete repro (no MLU/XPU hardware needed — this hits the plain CPU path) This isn't a hypothetical edge case. Confirmed live against real `verl` 0.9.1, with no hardware plugin logic other than MLU's involved at all: ``` >>> get_torch_profiler(contents=["cpu"], save_path=..., profile_step=3) TypeError: _patch_get_torch_profiler.<locals>._mlu_get_torch_profiler() got an unexpected keyword argument 'profile_step' ``` `profile_step` is not a made-up argument — it's what verl-core's own `Profiler.start()` passes on **every profiled step, on every platform** (`verl/utils/profiler/torch_profile.py`, call sites around lines 340 and 452). MLU's patch replaces `get_torch_profiler` process-wide at import time with a hand-copied version that never learned about this param, so any workload with the profiler enabled — CUDA, CPU, XPU, doesn't matter which — TypeErrors, purely because MLU support is installed. `tests/profilers/test_torch_profile_mlu.py` (added in this PR) is a red/green demonstration of exactly this. To see both sides yourself: ```bash pytest -q tests/profilers/test_torch_profile_mlu.py # passes on this branch git show origin/main:verl_hardware_plugin/profilers/torch_profile_mlu.py \ > verl_hardware_plugin/profilers/torch_profile_mlu.py pytest -q tests/profilers/test_torch_profile_mlu.py # fails on main's version, same TypeError as above git checkout -- verl_hardware_plugin/profilers/torch_profile_mlu.py # restore the fix ``` (The whitelist half of the fix shares the same root cause — reimplementing instead of delegating — but isn't independently reproducible with a real value today, since MLU's hardcoded whitelist happens to still match upstream's current one. It's a structural fix for the next platform that adds a content type, not a live crash yet.) ## Independent verification against clean upstream verl (no fork, pinned commit) Re-ran the repro from scratch against an unmodified `verl-project/verl` checkout, pinned to commit [`9097cc4`](verl-project/verl@9097cc4) (2026-08-19, #7408 "feat: add profiler post hook, fix behaviors and allow comprehensive torch profiler") — the commit that added the `profile_step`/`name_mini_batch_window` params this bug is about: ```bash # 1. verl-core, pinned to the commit that added profile_step/name_mini_batch_window git clone https://github.com/verl-project/verl cd verl && git checkout 9097cc4d4c9cf77e3fe88c34badd7d7549534e4a pip install -e . cd .. # 2. the plugin itself, this PR's branch gh pr checkout 29 --repo verl-project/verl-hardware-plugin pip install -e . ``` Standalone repro script (CPU only, no GPU/XPU/MLU hardware — drives the real `verl.utils.profiler.torch_profile.Profiler` class, the same one a trainer's training loop calls, not `get_torch_profiler` directly): https://gist.github.com/kahlun/74c8064c9f9f462df0fbfbda6a905dde - Against `main`'s `torch_profile_mlu.py`: same `TypeError: ... got an unexpected keyword argument 'profile_step'`, raised from inside verl-core's own `Profiler.start()` (`torch_profile.py:344`) — not the plugin or a test mock. - Against this branch: profiler starts/steps/stops, writes a real chrome trace, clean exit. Caveat for anyone re-verifying: the released `verl==0.9.0` PyPI wheel predates #7408, so it lacks `profile_step`/`name_mini_batch_window` — testing against that wheel won't reproduce the crash. Needs verl-core at or after `9097cc4`. ### Gotchas for anyone re-verifying on an XPU box - **Import order matters.** On torch 2.11+xpu, importing `verl` before `torch` makes Kineto's PTI backend fail `zeInit` and SIGABRT (exit 134) inside `prof.start()`, with stdout swallowed. This aborts deterministically whether or not the plugin is patched, so a naive control-vs-branch comparison looks damning until you notice the *unpatched* control aborts too. Pin `import torch` as the very first import. - **Post-fix, the XPU trace has zero device events.** CPU activity only — upstream `get_torch_profiler` has no XPU branch yet. "Works on XPU" here means no crash plus a CPU-side trace, not captured device kernels. ## The fix Stop reimplementing — delegate. Strip `"mlu"` out of `contents`/args, call the *saved original* `__post_init__` / `get_torch_profiler`, restore state after. MLU's own behavior when `"mlu"` is actually requested is unchanged; every other platform now goes through the real, up-to-date verl-core logic instead of a stale copy. ## Test plan - [x] `tests/profilers/test_torch_profile_mlu.py` added: fails against `main` with the `TypeError` above, passes against this branch (see repro steps above; also green in CI) - [x] Repro'd bug 2's `TypeError` on real B60 hardware with `profiler.tool_config.torch.contents=[xpu, cpu]` before this fix. Bug 1's `AssertionError` also fired in that run, but only with a local XPU content-name hook simulated (not present in stock verl or this plugin today) — see caveat above - [x] Same run produces a real Chrome trace after this fix - [x] Independently re-verified against clean upstream `verl-project/verl` (pinned commit, no fork) — see above - [ ] MLU's own `contents=[mlu, ...]` path re-verified on MLU hardware (need MLU access to confirm no regression) ## Why this matters for XPU work This is the delegate-not-reimplement pattern I'll reuse for the XPU torch.profiler monkey-patch (interim, until `torch_profiler_activity()`/`torch_profiler_content_name()` hooks land via verl-project/verl#7988). Landing this first makes the shared functions safe to chain against.
…in get_torch_profiler
Signed-off-by: kahlun <kah.lun.teoh@intel.com>
…lity in get_torch_profiler
…nt name validation
kahlun
marked this pull request as ready for review
October 6, 2026 02:44
kahlun
added a commit
to kahlun/verl-hardware-plugin-fork
that referenced
this pull request
Oct 6, 2026
Implements PlatformXPU.torch_profiler_activity()/torch_profiler_content_name(), paired with the new optional PlatformBase hooks proposed on the verl-core side (verl-project/verl#7988). Lets profiler.tool=torch's contents accept "xpu" and collect ProfilerActivity.XPU, the same shape CUDA/MLU already get, as a self-contained-trace-file alternative to the existing VTune/ITT path, which stays untouched. Verified on real B60 hardware: a torch.profiler run with contents=[xpu,cpu] produces a populated Chrome trace containing genuine Intel Level Zero driver activity (xpu_driver/xpu_runtime categories, zeDeviceGetRootDevice etc.).
kahlun
marked this pull request as draft
October 8, 2026 08:57
wuxibin89
added a commit
that referenced
this pull request
Oct 8, 2026
…orm hook and share the attention padding helpers (#7917) ### What does this PR do? Two independent changes, both making verl's device handling less CUDA-specific. **1. A `profiler_markers()` hook on `PlatformBase`.** Lets a hardware backend supply its own tracing markers -- `(mark_start_range, mark_end_range, mark_annotate, marked_timer)` -- instead of nvtx or mstx. Default is `None`, so existing platforms are unaffected. This also fixes marker resolution. `verl/utils/profiler/__init__.py` checked `is_nvtx_available()` before consulting the platform, and nvtx ships in the `verl-core` extra, so on any install where nvtx imports the platform was never asked. Order is now platform, nvtx, mstx, generic, and stays lazy so the platform need not exist at import time. Relatedly, `engine_workers.py` converts `tool_config` for any configured `profiler.tool` rather than an allowlist of the five built-in names. `omega_conf_to_dataclass` returns `None` for a missing or empty config, so a tool with no `tool_config` entry still works, and an out-of-tree tool can name its own dataclass through `_target_`. **2. The attention padding helpers become device-neutral.** `npu_flash_attn_utils.py` was already a pure torch/einops port of flash-attn's `bert_padding`, with nothing NPU-specific in it, so it is renamed `attention_padding_utils.py` and used as the fallback for *any* install where `flash_attn` will not import -- not just NPU. That covers non-CUDA accelerators and CPU-only installs such as unit tests and dev boxes. The duplicate `_fallback_*` helpers in `attention_utils.py` are deleted, and `torch_functional.py`'s two remaining hardcoded `flash_attn` / `get_device_name() == "cuda"/"npu"` branches now go through the same dispatcher. No public API changes. Consumed by `verl-hardware-plugin`'s `PlatformXPU` ([#22](verl-project/verl-hardware-plugin#22)). ### Why this is not duplicating an existing PR Searched open and closed PRs for `profiler_markers`, `PlatformBase hook`, `marked_timer`, `mark_start_range`, `attention_utils`, `npu_flash_attn_utils`, `index_first_axis`. For the profiler hook, nothing else lets a platform override the markers. #7988 (also mine) adds a different `PlatformBase` hook -- an optional `torch.profiler` activity list, i.e. what the built-in torch profiler records, not which marker functions the trainer calls. It overlaps only in touching `platform_base.py`; I will rebase whichever lands second. #6086 (merged) created the platform abstraction both extend. For the attention change, **#7098 overlaps substantially and predates this work.** It reaches the same conclusion from the same reasoning -- that the NPU copy only uses torch and einops, so dispatching on hardware was unnecessary -- and both PRs delete the dispatch and touch `tests/utils/test_attention_utils.py`, so they conflict. The differences are small: #7098 inlines the helpers into `attention_utils.py` and deletes the module, where this PR keeps it as `attention_padding_utils.py`; #7098 also normalizes indices to `int64`, which this PR does not. #7098 has the stronger test evidence of the two. I did not see it before implementing, and I am happy to drop part 2 here and review #7098 instead if you would rather land that -- part 1 is independent and does not depend on it. ### Test ```bash python -m pytest -q tests/utils/test_profiler_markers.py python -m pytest -q tests/utils/test_attention_utils.py python -m pytest -q tests/plugin/test_platform_abstraction.py ``` | Target | Result | | --- | --- | | `tests/utils/test_profiler_markers.py` | 2 passed, 2 skipped | | `tests/utils/test_attention_utils.py` | 5 passed | | `tests/plugin/test_platform_abstraction.py` | 14 passed | The 2 skips are the nvtx and mstx paths; neither package is installed in this image. Both new unit test files inject a fake platform, so no accelerator is needed. Against a real out-of-tree platform supplying Intel ITT markers, `_resolve_markers()` selects them even with `is_nvtx_available()` forced to `True` -- the regression the fix targets. `profiler.tool` resolution was checked for three cases: an out-of-tree tool name resolves to its implementation, a built-in name still reaches core's dispatch, and an unknown name still falls through to `_NoOpProfiler`. #### Existing tests touched Four existing test files change only in routing their `unpad_input` / `pad_input` imports through `verl.utils.attention_utils` -- no logic change: - `tests/models/test_transformer.py` - `tests/models/test_transformers_ulysses.py` - `tests/utils/test_activation_offload.py` - `tests/special_distributed/test_fsdp_ckpt.py` They are covered by the existing CUDA/NPU CI (`model.yml`, `model_ascend.yml`). Before this change they failed at import on a device without a flash-attn build (`unpad_input` unbound). Results on 4x Intel Arc Pro B60 24GB: - `test_transformer.py` -- 4/4 passed - `test_transformers_ulysses.py` (`torchrun --nproc_per_node=4 -m pytest`) -- all `sp_size<=4` configs pass; `sp_size=8` needs 8 GPUs - `test_activation_offload.py` -- fsdp2 cases pass; FSDP1 cases hit a Level Zero runtime error, an XPU-side issue unrelated to this change - `test_fsdp_ckpt.py` -- the default config exceeds 24GB per device; with a reduced batch, fsdp2 save/load round-trips bit-exactly ### Design & Code Changes | File | Change | | --- | --- | | `verl/plugin/platform/platform_base.py` | `profiler_markers()`, `None` by default | | `verl/utils/profiler/__init__.py` | resolution order and the nvtx-shadowing fix | | `verl/workers/engine_workers.py` | allowlist replaced with an any-tool check | | `verl/utils/npu_flash_attn_utils.py` -> `verl/utils/attention_padding_utils.py` | rename only | | `verl/utils/attention_utils.py` | falls back to the padding utils whenever `flash_attn` will not import; duplicate `_fallback_*` helpers deleted | | `verl/utils/torch_functional.py` | two hardcoded device branches routed through the dispatcher | | `tests/utils/test_profiler_markers.py`, `tests/utils/test_attention_utils.py` | new | ### Checklist Before Submitting - [x] Read the [Contribute Guide](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md). - [x] Apply [pre-commit checks](https://github.com/verl-project/verl/blob/main/CONTRIBUTING.md#code-linting-and-formatting). - [ ] Add / Update [the documentation](https://github.com/verl-project/verl/tree/main/docs). - [x] Add unit or end-to-end test(s) to [the CI workflow](https://github.com/verl-project/verl/tree/main/.github/workflows). The hardware that exposes the original collision is not in CI; the in-tree platforms are covered by unit tests and the table above documents that their behavior is unchanged. - [x] Once your PR is ready for CI, send a message in the `ci-request` channel. - [x] If your PR is related to the `recipe` submodule — not applicable. --------- Co-authored-by: wuxibin <wuxibin@bytedance.com>
wuxibin89
pushed a commit
that referenced
this pull request
Oct 9, 2026
…t other pytorch supported device (#8151) ### What does this PR do? `get_torch_profiler` knew exactly one device activity, `ProfilerActivity.CUDA`, and `TorchProfilerToolConfig` accepted exactly one device keyword, `"cuda"`. On an accelerator that is not CUDA there was then no way to record device kernels: the default `contents=[]` asks for CUDA, torch logs `CUDA is not available, disabling CUDA profiling` and the trace holds CPU ops only, while naming the real device in `contents` fails config validation. torch exposes one `ProfilerActivity` member per backend it can profile, named after the device type it profiles, and a single profiler session records exactly one of them. So the active platform's `device_name` is both the activity to collect and the `contents` keyword that requests it. This PR resolves it that way instead of hardcoding CUDA, and takes the accepted device keywords from `torch.profiler.ProfilerActivity` rather than from a verl-maintained list. **No new `PlatformBase` API** — `device_name` is already an abstract property. ### No change for existing platforms `PlatformCUDA.device_name` is `"cuda"`, and `PlatformROCm`, `PlatformMetaX` and `PlatformIluvatar` inherit it (torch exposes HIP as `torch.cuda`), so all four build exactly the same `activities` list as before. Platforms with no matching member (`npu`, `mlu`, `musa`, `gcu`, `tpu`, `supa`) still get a CPU-only trace, exactly as they do today when torch disables the unavailable CUDA activity — the warning explaining it is now verl's, and it names the device. `xpu`, `hpu` and `mtia` start working with no further verl change and no plugin code. ### Out-of-tree backends whose activity is not named after the device type `DEVICE_ACTIVITIES` is a module-level `device type -> ProfilerActivity` map, so a backend that needs a different pairing can add one entry from its plugin: ```python from verl.utils.profiler.config import DEVICE_ACTIVITIES DEVICE_ACTIVITIES["mlu"] = torch.profiler.ProfilerActivity.PrivateUse1 ``` This is deliberately data rather than API: it keeps the mapping out of `PlatformBase` while leaving privateuse1-based backends a one-line override. In practice those backends all select a vendor profiler through `global_profiler.tool` instead, which this PR does not touch. `CPU` and `PrivateUse1` are excluded from the map and `"privateuse1"` is not an accepted `contents` keyword: CPU activity is collected in every trace regardless of `contents`, and `PrivateUse1` is torch's slot for an out-of-tree backend rather than a device type any platform reports as its `device_name`, so accepting it would validate a value that is then silently never recorded. `"cpu"` remains accepted. ### Why this is not a duplicate - #7988 proposed the same fix through two optional `PlatformBase` hooks, `torch_profiler_activity()` and `torch_profiler_content_name()`. This PR drops both: `device_name` already carries the information, which removes the paired-hook invariant between them, removes the requirement for every plugin to implement them, and leaves `platform_base.py` untouched. - `gh pr list --repo verl-project/verl --state open --search "profiler activity"` — nothing else open in this area. ### Test ```bash pytest tests/utils/test_torch_profile.py # 50 passed, 18 subtests passed VERL_PLATFORM=intel pytest tests/utils/test_torch_profile.py # 50 passed, 18 subtests VERL_PLATFORM=cambricon pytest tests/utils/test_torch_profile.py # 50 passed, 18 subtests ruff check verl/utils/profiler/config.py verl/utils/profiler/torch_profile.py \ tests/utils/test_torch_profile.py # clean ruff format --check <same three files> # clean ``` New tests: - `TestDeviceActivityFromPlatform` — a non-CUDA device is recorded and CUDA is *not* added alongside it (torch checks CUDA before every other device, so leaving it in would disable device profiling altogether); CUDA platforms are unchanged; every entry in `DEVICE_ACTIVITIES` resolves to its own activity, so the lookup holds for whatever members a given torch build exposes; a device with no member warns; a CUDA-written `contents` run on another accelerator warns and records no device activity; `contents` naming only profiler options warns nothing; a CPU platform does not add CPU twice. The non-CUDA cases take their device from whatever `torch.profiler.ProfilerActivity` the running build exposes rather than hardcoding one accelerator, and skip if a build offers none. - `TestDeviceContentsConfigValidation` — every keyword in `DEVICE_CONTENTS` validates, profiler options still validate, an unknown keyword and `"privateuse1"` are both rejected. Two pre-existing tests (`test_get_torch_profiler`, `test_cpu_activity_added_for_device_only_contents`) assert the CUDA activity, which until now was hardcoded and so held on any machine. They now patch in a CUDA platform, stating the assumption they always had — without it they fail when the suite runs on non-CUDA hardware. No new e2e CI test: CI has no non-CUDA device. Manually verified end to end on Intel Arc Pro B60 (device kernels present in the chrome trace, which was not the case before this change). ### API and Usage Example No API change. On a non-CUDA accelerator, the default config now records device kernels: ```yaml global_profiler: tool: torch global_tool_config: torch: contents: [] # records the active platform's device activity, e.g. xpu # contents: [xpu] # equivalent, explicit # contents: [stack] # CPU-only, as requested, no warning ``` ### Design & Code Changes - `verl/utils/profiler/config.py` — `DEVICE_ACTIVITIES` map built from `torch.profiler.ProfilerActivity.__members__` (keyed by lowercased device type, so the lookup does not depend on how torch capitalizes a member); `DEVICE_CONTENTS` derived from it for config validation, replacing the hardcoded `["cuda", "cpu", "memory", "shapes", "stack"]`. - `verl/utils/profiler/torch_profile.py` — `get_torch_profiler` resolves the active platform's `device_name` through that map; warns when the platform has no activity, and when `contents` names a device activity belonging to a different backend. - `docs/perf/torch_profiling.md` — documents the device keyword per accelerator. - `tests/utils/test_torch_profile.py` — as above. ### Checklist - [x] Searched for similar PRs (see "Why this is not a duplicate"). - [x] Title follows `[{modules}] {type}: {description}`. - [x] `pre-commit` / ruff clean on changed files. - [x] Docs updated (`docs/perf/torch_profiling.md`). - [x] New CI-runnable unit tests added. - [ ] Slack `ci-request` ping if CI needs a nudge. - [ ] Recipe submodule unchanged (n/a).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently moved from Ready to Draft.
By trying the mechanism of no new API mode, try to utilize the platform_base.
What does this PR do?
Lets a hardware platform plugin add its own device activity to verl's torch profiler (
global_profiler.tool=torch).Today
get_torch_profilerknows only one device activity,ProfilerActivity.CUDA. PyTorch also has activities for other backends (e.g.XPU,HPU,MTIA), but verl has no way to request them. CUDA itself works fine on NVIDIA (and AMD ROCm, which reuses the CUDA activity). The problem is only on machines with a different accelerator and no CUDA device:contents=[], verl asks forCUDA, torch warnsCUDA is not available, disabling CUDA profiling, and the trace has CPU ops but no device kernels;contentsfails config validation, because onlycuda/cpu/memory/shapes/stackare accepted.This PR adds two optional hooks to
PlatformBase. A plugin uses them to tell the profiler which activity to record and whatcontentsname turns it on. Both hooks default toNone, so CUDA, ROCm and NPU behave exactly as they do now.Related: #7408, #7099 (torch profiler rework).
Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI)Test
tests/plugin/test_platform_abstraction.py::TestTorchProfilerHookschecks that the default hooks returnNone, which is the path every in-tree platform takes.cuda,rocm,npu) overrides the hooks, soget_torch_profilerbuilds the sameactivitieslist as before.tests/utils/test_torch_profile.py::TestPluginProfilerActivityHookandTestPluginContentNameConfigValidationcover the plugin path with a mocked platform — activity added for empty/matchingcontents, excluded for non-matchingcontentsor when either hook returnsNone, and the regression case where an explicitcontents=["cuda"]must still get real CUDA on a platform that also exposes a plugin hook (the plugin device must be requested, not just available).contentsnames both"cuda"and the plugin's keyword — only one activity is ever recorded (the plugin's), andget_torch_profilernow logs a warning saying so, instead of silently dropping"cuda".contents=[],["xpu"], and["cuda","xpu"]alike (XPU wins in the last case). Without the hooks, it contains CPU ops only.No new e2e test is added to CI — the real-hardware path needs a non-CUDA device and an out-of-tree plugin, which CI has neither. The plugin path itself is now covered by mocked unit tests, so CI gets real coverage of the hook contract even without that hardware.
API and Usage Example
A platform plugin opts in by overriding both hooks:
User config does not change. An empty
contentsrecords CPU plus the plugin's device, just as an emptycontentsrecords CPU plus CUDA today. The plugin's name can also be listed explicitly:Design & Code Changes
How other hardware is supported. The hooks map onto how
torch.profileralready works. A single profiler session records one device backend, chosen fromactivities(_KinetoProfile.__init__). Each plugin therefore provides exactly one activity, plus a name for it incontents. Any backend that has aProfilerActivitymember can use this without changes in verl:CUDACUDA(HIP is exposed astorch.cuda)npuprofiler toolXPU/HPU/MTIAWhy CUDA is left out when a plugin device is active.
torch.profilerchecks forCUDAbefore every other activity. IfCUDAis in the list on a machine without CUDA, torch turns off device profiling completely, which would drop the plugin's activity too. So when a plugin's device is actually requested —contentsis empty, or contains the plugin's keyword — verl adds that activity instead ofCUDA. The key word is requested: a platform merely exposing the hooks does not by itself suppress CUDA —contents=["cuda"]alone still gets realCUDA, even on a platform that also implements the hooks. CUDA machines are unaffected either way, because no CUDA-family platform overrides the hooks.If
contentsnames both"cuda"and the plugin's keyword in the same call — a self-contradictory request, since only one device activity is ever recorded and there's no CUDA device on a plugin platform to begin with — the plugin's activity wins andget_torch_profilerlogs a warning naming which activity was kept and that"cuda"was ignored.Changes
verl/plugin/platform/platform_base.py: newtorch_profiler_activity()andtorch_profiler_content_name(), both returningNoneby default. A platform opts in only when both are non-None.verl/utils/profiler/config.py:TorchProfilerToolConfigvalidation accepts the plugin's content name when both hooks are set.verl/utils/profiler/torch_profile.py:get_torch_profileradds the plugin activity whencontentsis empty or includes the plugin's name, and addsCUDAonly when no plugin device is present.verl/utils/profiler/torch_profile.py:get_torch_profileradds the plugin activity whencontentsis empty or includes the plugin's name, and addsCUDAonly when the plugin device was not actually requested (not merely available); logs a warning whencontentsnames both"cuda"and the plugin's keyword, since only the plugin's activity is recorded.tests/utils/test_torch_profile.py:TestPluginProfilerActivityHookandTestPluginContentNameConfigValidationcover the plugin path, thecontents=["cuda"]-not-stolen regression, and the new conflicting-selection warning.verl/plugin/platform/README.md: documents both hooks in the platform API table.tests/plugin/test_platform_abstraction.py: tests the default behavior.Checklist Before Submitting
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysNone) path is unit-tested. The plugin path needs a non-CUDA device and an out-of-tree plugin, and CI has neither (see Test).ci-requestchannel in theverlSlack workspace.recipesubmodule, please also update the reference to the submodule commit viagit submodule update --remoteorcd recipe && git pull origin main.