Repository navigation
[XPU] feat(xpu): add VTune (ITT) profiler backend - #22
Merged
physics31415926 merged 23 commits intoOct 13, 2026
Merged
Conversation
…uler.avg override behavior, attention utils point to existing npu file
docs/user_guide_xpu/README.md already listed these three files in its "Directory Structure" section but none of them existed. Adapted from docs/intel_gpu_tutorial/ (verl-project/verl, fork/verl working tree, uncommitted) with corrections: verl_xpu -> verl_hardware_plugin (this repo's actual package name), VERL_USE_EXTERNAL_MODULES -> entry_points auto-discovery (this plugin declares verl.plugins entry_points in pyproject.toml, no env var needed), and Dockerfile references pointed at docker/intel_gpu/ in this repo instead of verl-core's.
FSDPEngineWithLMHead/FSDPEngineWithValueHead (verl-core, verl/workers/engine/fsdp/transformer_impl.py) expose the wrapped model as self.module, never self.model. Both XPU engine subclasses called self.model in initialize(), which isn't caught by the hasattr() guard -- hasattr() only suppresses AttributeError from its own internal getattr, not from evaluating self.model as an argument, so this raised immediately and crashed ref-model init before training could start. Confirmed on real B60 hardware, single GPU, both FSDP1 and FSDP2 (run_grpo_intel_gpu.sh / run_grpo_fsdp2_intel_gpu.sh): identical 'FSDPXPUEngineWithLMHead' object has no attribute 'model' crash in WorkerDict.actor_rollout_ref_init_model() -> engine.initialize(), for both strategies -- this predates the vLLM v0.29.0 bump and the VtuneProfiler/hook work in this PR, it's a pre-existing bug in the base engine file.
Not ready to ship yet -- pulling it out rather than leaving stale mentions in quick_start.md and the pre-existing README.md stub. Also removes the RAY_memory_monitor_refresh_ms/RAY_NUM_PRESTART_PYTHON_WORKERS example from quick_start.md's launch snippet for the same reason as the 8-GPU script: only validated at 2 GPUs, disables a real Ray safety net.
install_guidance.md said nothing about the fact that is_reduce_avg_supported/attention_utils_module/profiler_markers/ dist_profiler_cls only take effect on a verl-core build that includes the still-unmerged verl-project/verl#7917 - against stock main they silently no-op instead of erroring. Also add the missing VTune row to quick_start.md's feature matrix.
Restructure README/install_guidance/quick_start to match the other vendor guides' level of detail: source-tree listing in the README, a full Environment Variables table, prerequisites-before-install ordering, numbered install/quick-start steps, a platform-resolution sanity check before the full training run, the plugin test-suite command, and Next Steps/Related Documentation sections. No new technical claims - same facts as before, just organized and filled out to match user_guide_tpu's structure.
5 of 6 tasks
Fills the last missing user_guide_xpu doc relative to user_guide_mlu's four-file structure. Documents the actual dist_profiler_cls/tool_config wiring verified against a live Hydra compose (there is no generated tool_config.vtune schema entry, unlike nsys/npu/torch -- it must be added via +tool_config.vtune._target_=NsightToolConfig), and clarifies that VTune attaches externally as a collector rather than writing a self-contained trace file the way nsys/torch/npu do.
Two mypy errors failed the lint job: - marked_timer's `color: str = None` is an implicit Optional (PEP 484); annotate it Optional[str] to match the sibling mark_* helpers. - `tool_config.discrete` is read unconditionally even though tool_config is Optional. Beyond the union-attr error this is a live AttributeError: when the profiler is disabled, tool_config is None and the preceding assert passes, so construction crashes. Default discrete to False in that case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`profiler.tool=vtune` crashed every worker unless the user hand-added a
`+tool_config.vtune.*` Hydra override, including when `profiler.enable`
was False.
verl core has no `tool_config.vtune` schema entry, so the caller's
`tool_config.get("vtune")` resolves to None. DistProfiler.__init__ then
substitutes the *whole* tool_config mapping for that None:
if tool_config is None:
tool_config = config.tool_config
That mapping is non-empty, so the old `if not tool_config` guard did not
fire and the `else False` fallback never engaged — but it is a plain
dict/DictConfig with no `discrete` key, so `tool_config.discrete` raised
`AttributeError: 'dict' object has no attribute 'discrete'`.
Read it with `getattr(..., "discrete", False)` instead, mirroring what
core itself does at verl/utils/profiler/profile.py (`self._discrete =
getattr(tool_config, "discrete", False) if tool_config else False`).
False is always the correct default on XPU: `discrete` only gates
`PlatformXPU.profiler_start`/`profiler_stop`, which are no-ops here, so
both values behave identically.
The `assert not config.enable` is dropped as dead code — tool_config is
never falsy on the real resolution path, so the branch was unreachable
in the enable=True case it was meant to catch.
Explicit `+tool_config.vtune.discrete=True` overrides are still honoured.
Test: tests/test_plugin_registration.py::TestVtuneProfilerToolConfig
covers tool_config of None / {} / the full mapping core actually passes,
at both enable values, plus an explicit NsightToolConfig(discrete=True).
4 of the 6 default-path cases fail before this change (2 AssertionError,
2 AttributeError) and all pass after.
The VTune guide told users that omitting `+tool_config.vtune.*` raises `AssertionError: tool_config must be provided when profiler is enabled` and that this was "intentional, not a bug to work around". Both halves were wrong: the real failure was an AttributeError from a different repo's code path, and it was a bug (fixed in the preceding commit). Lead with the minimal recipe — `profiler.tool=vtune` plus the usual enable/ranks keys — and keep the `+tool_config.vtune.*` form as an optional explicit entry, noting it changes nothing functionally on XPU because `discrete` only gates the no-op profiler_start/profiler_stop hooks. Drop the now-unreachable AssertionError troubleshooting bullet.
5 of 6 tasks
…odule fix Per verl-hardware-plugin#26, the maintainer's ruling (2026-09-23) is to default new hardware backends to plugin-side monkeypatching, reserving new verl-core PlatformBase hooks for genuine hardware-agnostic blockers. The xccl reduce_avg workaround doesn't need one -- it's already applied entirely from this plugin's own fsdp_xpu.py engine (self.module. set_force_sum_reduction_for_comms(True) after super().initialize()), with no PlatformBase hook involved. Dropping is_reduce_avg_supported() here so this PR only asks verl-project/verl#7917 for the two hooks XPU genuinely can't provide on its own: attention_utils_module() and the profiler hooks. Keeps the self.model -> self.module fix in fsdp_xpu.py -- that's an independent, hardware-confirmed bug (FSDPEngineWithLMHead/WithValueHead expose the wrapped model as self.module, never self.model) that has nothing to do with the hook-vs-monkeypatch question. Follow-up plugin-side monkeypatch work for the remaining raw dist.all_reduce(op=AVG) call sites lives on experiment/xpu-monkeypatch-reduce-avg-attention.
5 of 9 tasks
…erl-project#32 Redundant scope: this PR no longer touches fsdp_xpu.py for any AVG-related reason (is_reduce_avg_supported was dropped in the previous commit), and that file's only remaining purpose here was this one attribute fix, which lives entirely in the pre-existing (pre-#22) reduce_avg workaround code. experiment/xpu-monkeypatch-reduce-avg-attention already carries the same fix as part of a strict superset (root + every per-layer FSDP2 wrapper, vs. this PR's root-only version) -- keeping both risks two PRs racing to fix the same lines. Leaving this PR to attention_utils_module and the VTune profiler hooks only.
kahlun
referenced
this pull request
in kahlun/verl-hardware-plugin-fork
Sep 24, 2026
…atch attention_utils_module() is staying a PlatformBase hook (see PR #22, and verl-project/verl#7917's updated description) -- it passed the maintainer's bar on verl-hardware-plugin#26 for a genuinely hardware-agnostic capability that couldn't be done cleanly from the plugin alone. Carrying a monkeypatch alternative for it here too just duplicates an already-accepted, working solution with no benefit. is_reduce_avg_supported() didn't pass that bar and is this branch's actual job: the xccl AVG gap can be fully handled from the plugin (FSDP2 via an engine-level method override, the 3 remaining raw dist.all_reduce(op=AVG) sites via a scoped, documented monkeypatch). Renamed the design doc to match (xpu-monkeypatch-reduce-avg.md) and folded in a note that this branch is now the sole owner of the fsdp_xpu.py self.model->self.module fix -- it's been dropped from #22's smaller, root-only version of the same fix now that #22 no longer touches this file for any other reason.
kahlun
changed the title
kahlun
referenced
this pull request
in kahlun/verl-hardware-plugin-fork
Sep 24, 2026
Per kahlun's review-thread investigation on verl-project/verl#7917 (measured on real 4xB60 hardware, not reasoned from source): xccl does not uniformly lack ReduceOp.AVG. oneCCL implements it on its SYCL-kernel execution path but not on its scheduler path; path selection is internal to oneCCL and isn't capability-aware, so an AVG request can abort mid-collective ("average operation is not supported for the scheduler path") rather than reliably succeed or reliably fail. Fix tracked for oneCCL 2022.2 / torch 2.15, not yet available on the stack this plugin targets. Also note the separate, narrower double-division bug on very small messages (intel/torch-xpu-ops#3020) is fixed on that same stack and is not what either the FSDP2 engine override or this patch works around -- easy to conflate the two since both are filed against xccl's AVG support. The blanket "xccl does not support ReduceOp.AVG" phrasing this replaces was accurate enough to justify the workaround but not accurate about why, and was written before this investigation existed. Same mechanism, same correctness -- corrected reasoning only. platform_xpu.py has the same blanket phrasing but predates both this branch and #22; left alone here since fixing it isn't part of this PR's diff. 76 passed.
- install_guidance.md: vLLM no longer needs to be built from source for XPU -- vLLM now publishes a prebuilt XPU wheel via wheels.vllm.ai, which docker/intel_gpu/Dockerfile.intel_gpu already uses instead of a from-source build. Update the prerequisite and Docker sections to match. - quick_start.md: trim the inline verl-core/#7917 explanation on the Profiler row down to a pointer at profiling.md (full story already lives in install_guidance.md/profiling.md), and standardize "not yet supported/validated/tested" phrasing to "Work in Progress" across the summary bullets and Feature Support Matrix. - README.md: apply the same "Work in Progress" phrasing to the Megatron engine directory-structure note; bump Last updated date.
physics31415926
pushed a commit
that referenced
this pull request
Sep 30, 2026
## Open question for reviewers, up front `reduce_avg_allreduce_patch.py` patches `torch.distributed.all_reduce` when `op=AVG` — the only mechanism available for verl-core's 3 raw `dist.all_reduce(op=ReduceOp.AVG)` call sites, since there's no per-site wrapper to hook into and this plugin doesn't own those call sites. It's scoped to `dist.get_backend(group) == "xccl"`, so a gloo/nccl group in the same process (e.g. a CPU-only Ray actor's coordination group alongside a GPU worker's xccl group) is untouched and keeps native `AVG` — this was originally unscoped (any group, any backend) and has been fixed to check the backend per call. What's left: anyone reading verl-core's source still can't see, from that source alone, that an xccl group's `ReduceOp.AVG` silently becomes SUM+divide, for as long as the process runs (there's no narrower call-boundary available — see Changes below for why). That's a real design smell, not a clean win, and is the one piece here still worth a narrow core-hook ask if that's judged unacceptable — happy to split it into its own PR against #7917 if so. ## Summary Plugin-side replacement for `is_reduce_avg_supported()` — implemented with **zero `verl-core` changes**: no new `PlatformBase` hook, no dependency on [verl-project/verl#7917](verl-project/verl#7917). ## Motivation [verl-hardware-plugin#26](#26) asked maintainers whether Intel XPU should extend `PlatformBase` in `verl-core` (the TPU precedent) or stay entirely inside this plugin via monkeypatch (the MLU precedent). The maintainer's answer (2026-09-23): default to the plugin, keep #7917 narrowed to only what truly can't be done from here. `is_reduce_avg_supported`/`all_reduce_avg` has just been dropped from #7917 and from [#22](#22) on that basis -- the underlying reduce_avg problem doesn't need a core hook to work around. (Correction from an earlier version of this PR: the reason isn't "xccl doesn't implement `ReduceOp.AVG`" -- per [this review-thread investigation](verl-project/verl#7917 (comment)) on real 4xB60 hardware, oneCCL implements `AVG` on its SYCL-kernel path but not its scheduler path, and path selection is internal and not capability-aware, so an `AVG` request can abort mid-collective rather than reliably succeed or fail. Fix tracked for oneCCL 2022.2 / torch 2.15. Same mechanism below, corrected reasoning.) This PR is the plugin-side replacement. `attention_utils_module()` and the profiler hooks passed the maintainer's bar and stay as `PlatformBase` hooks — implemented for XPU in #22, which this PR does not touch or duplicate. (An earlier draft of this branch also monkeypatched attention as a "how far can pure monkeypatch go" comparison; dropped once it was clear the hook was staying and a plugin-only alternative for it served no purpose.) ## Changes - `verl_hardware_plugin/engines/fsdp_xpu.py`: extend the existing (non-hook, non-patch) `initialize()` override to call `set_force_sum_reduction_for_comms(True)` on the FSDP2 root **and every individually-wrapped transformer layer**, not just the root — each `fully_shard()`'d module has independent FSDP state. This is ordinary method-override behavior on the plugin's own `EngineRegistry`-registered engine, not a monkeypatch, so it's robust regardless of import order. Also fixes an unrelated `self.model`→`self.module` attribute bug in the same function (`FSDPEngineWithLMHead`/`WithValueHead` expose the wrapped model as `self.module`, never `self.model`) — #22 had picked up the same fix independently since it lives in the same lines; dropped from #22 in favor of this PR's superset version (root + per-layer, vs. root-only). - `verl_hardware_plugin/patches/reduce_avg_allreduce_patch` (new): monkeypatches `torch.distributed.all_reduce(op=AVG)` → `SUM` + manual divide, scoped to `xccl` process groups (`dist.get_backend(group) == "xccl"`), for the three remaining raw `dist.all_reduce(op=ReduceOp.AVG)` call sites in verl-core (`sft_trainer.py`, `engine_workers.py`, `profiler/performance.py::reduce_timing`). See the open question above. `async_op=True` call sites are intentionally left unpatched (falls through to original behavior) rather than risk returning a wrong result before the collective completes. - `verl_hardware_plugin/patches/__init__.py` (new): plain package docstring, no code — holds `reduce_avg_allreduce_patch` as the one patch module. No generic `apply_all()` aggregator: it had exactly one caller and one module, so it wasn't earning its keep (see the platform_xpu.py bullet below). - `verl_hardware_plugin/platforms/platform_xpu.py`: `PlatformXPU.__init__` calls `reduce_avg_allreduce_patch.apply()` directly. **Deliberately not called from the shared `verl_hardware_plugin/__init__.py`** (that file is common to every vendor this plugin supports) — `PlatformXPU()` is only constructed once `verl.plugin.platform.platform_manager._create_platform()` has actually selected `"intel"` as the platform for this process. Calling it from the shared top-level `__init__.py` instead would fire on any host where `torch.xpu` merely happens to be importable, even if a different platform ends up selected for the run — e.g. a mixed host with `VERL_PLATFORM=nvidia` explicitly set would still get its process-wide `all_reduce` semantics changed for no reason. (Two earlier drafts of this PR had this wired the wrong way, then routed through an unnecessary same-package `apply_all()` wrapper — both fixed.) ## Testing - [x] `pytest tests/ -v` passes (76 passed), including new coverage: - `PlatformXPU()` instantiation triggers `reduce_avg_allreduce_patch.apply()`; the shared plugin `__init__.py` no longer binds any patch call directly (regression guard against re-introducing the wrong wiring). - `reduce_avg_allreduce_patch`: `op=AVG` on an `xccl` group → `SUM` + divide by world size; `op=AVG` on a non-`xccl` group (e.g. `gloo`) passes through untouched; other ops pass through unchanged; `async_op=True` + `AVG` falls through unpatched; no-op when XPU unavailable; idempotent (calling `apply()` twice doesn't double-wrap). - [x] Manually verified on target hardware (B60): What's new *here* is the method-override framing - `reduce_avg_allreduce_patch.py`'s actual collective behavior is unit-tested ## Acceptance Baseline (for new hardware adaptation PRs) - [ ] Ran `scripts/baseline_grpo_gsm8k.sh` on target hardware (8 devices) - [ ] Training completed all epochs without error - [ ] `critic/rewards/mean` shows clear upward trend in first 100 steps - [ ] Curve is consistent with NVIDIA reference ## Checklist - [x] Code follows the project's style and passes `pre-commit` checks - [x] Documentation updated — folded into this description rather than a `docs/design/` file; this repo has no precedent for keeping RFC/experiment-verdict docs in the merged tree - [x] No secrets or credentials included --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…rface Tracks the rework of verl-project/verl#7917, which cuts the PlatformBase additions down to `profiler_markers()` alone. - Drop `attention_utils_module()`. Nothing is needed from the plugin here: verl core now routes every backend without a `flash_attn` build through its own pure-PyTorch `attention_padding_utils` (the former `npu_flash_attn_utils`, renamed device-neutral), which is the same bert_padding port this hook was pointing at. - Drop `dist_profiler_cls()`. `VtuneProfiler` is now registered for `profiler.tool: vtune` by a monkeypatch over `DistProfiler.__init__` (profilers/register_vtune.py) instead of a core hook. verl core has no profiler registry, so the hook was adding a second dispatch mechanism alongside its hardcoded if/elif; per the ruling on #26 a plugin-side monkeypatch is the default for this kind of thing. The patch is double-gated: applied from `PlatformXPU.__init__` rather than at import time, and behind a live `torch.xpu.is_available()` check, because verl's platform auto-detection instantiates every registered platform to probe it -- construction alone does not mean XPU was selected. - Keep `profiler_markers()`. Core's module-level markers are imported by the trainer at module scope, so there is no plugin-side seam for them. Docs updated: only one hook now depends on an unmerged verl-core PR, and the `tool_config` allowlist lift in engine_workers.py is called out as the other thing #7917 is needed for.
…-attention # Conflicts: # tests/test_plugin_registration.py # verl_hardware_plugin/platforms/platform_xpu.py
The registration patch self-gates on a live XPU device, matching reduce_avg_allreduce_patch.apply(), so the tests stub that probe the same way the reduce-avg suite does, and assert the no-device path leaves DistProfiler untouched.
kahlun
changed the title
kahlun
force-pushed
the
feature/xpu-vtune-avg-attention
branch
from
October 8, 2026 06:22
cc678bb to
2cf1b97
Compare
… availability The guide didn't say that a VTune collector is a separate install from the ITT markers this plugin emits, which makes a missing collector look like broken plugin support. ITT is notify-only -- markers are discarded when nothing has attached -- so the two failure modes are indistinguishable without being told. Verified on 1x Arc Pro B60: torch.profiler.itt.is_available() is True on torch 2.13.0+xpu and torch._C._itt exposes rangePush/rangePop/mark, so the marker path is live; but the docker/intel_gpu image's oneAPI 2026.1 install ships no vtune component and no libittnotify, so no capture is possible there. - Prerequisites: new bullet on the collector being a separate oneAPI component, absent from the image, plus the sampling permissions a container needs. - Troubleshooting: how to tell a missing collector from broken ITT bindings.
wuxibin89
added a commit
to verl-project/verl
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>
Resolve the accelerator reorganization (upstream verl-project#41) against this branch: - Move the XPU profiler modules under verl_hardware_plugin/accelerators/xpu/profilers/ (git's rename detection had guessed accelerators/mlu/profilers/) and add the package __init__. - Drop the now-deleted shared verl_hardware_plugin/profilers package; the only consumer was PlatformXPU.profiler_markers(). - Repoint every import of verl_hardware_plugin.profilers.* in platform_xpu, register_vtune and tests/test_plugin_registration.py. - Keep upstream's platform_xpu.__init__ comment and add the vtune registration patch alongside the reduce_avg patch. - Move the XPU docs to docs/accelerators/xpu/ and fix link depth (../development.md -> ../../development.md, ../user_guide_mlu/ -> ../mlu/). The docker/intel_gpu/ links were relative to a directory that does not exist in this repo at all -- it lives in the kahlun/verl fork, so they now point there explicitly. - Refresh the directory tree in docs/accelerators/xpu/README.md. Also corrects two comments the hook narrowing left stale: reduce_avg_allreduce_patch no longer claims verl#7917 ships an is_reduce_avg_supported() hook (it was dropped), and install_guidance no longer attributes the xccl reduce_avg workaround to fsdp_xpu.py.
profiler_markers() is the only PlatformBase hook this PR implements and it had no test, so the accelerator reorganization could have broken its lazy import of itt_profile_xpu without the suite noticing. Assert the arity and order core unpacks positionally, and that all four markers are callable no-ops when no ITT collector is attached.
kahlun
marked this pull request as ready for review
October 9, 2026 03:24
…r links - profiling.md's intro compared ITT support to Cambricon MLU's profiling path; removed, it added nothing XPU-specific. - #7917 (the profiler_markers() hook) merged into verl-core main on 2026-10-08. All four docs still called it 'open, not yet merged' and described a degraded fallback path as the default; updated to the merged state, including quick_start.md's feature matrix (Profiler row was 'Work in Progress'). - docker/intel_gpu/ doesn't exist on any verl main (fork or upstream) -- it's only in open PR verl-project/verl#7371. Every link previously pointed at the kahlun/verl fork tree, implying it was already merged there; repointed to the PR itself and noted it's still unmerged.
…ferences to unmerged Docker image PR
Collaborator
|
please resolve the conflicts |
…vg-attention Resolve the test_plugin_registration.py conflict by keeping both the incoming TestReduceAvgAllReducePatchTPU tests from main and this branch's TestProfilerMarkersHook/TestVtuneProfilerToolConfig/TestVtuneProfilerRegistration tests.
Collaborator
Author
@physics31415926 |
physics31415926
approved these changes
Oct 13, 2026
askhat-g
pushed a commit
to askhat-g/verl-hardware-plugin
that referenced
this pull request
Nov 1, 2026
## Summary Adds Intel VTune (ITT) profiling support for Intel XPU, plus the XPU user guide that fills the stub `docs/accelerators/xpu/README.md`. After the rework described below, this PR implements exactly **one** `PlatformBase` hook — `profiler_markers()` — and does everything else from the plugin side: - `PlatformXPU.profiler_markers()` → ITT range markers, so verl core can emit trace ranges without knowing about ITT or XPU. - `VtuneProfiler(DistProfiler)`, selected by `profiler.tool: vtune`, registered by a plugin-side monkeypatch over `DistProfiler.__init__` (`verl_hardware_plugin/accelerators/xpu/profilers/register_vtune.py`) — no core hook. ## What changed since this PR was opened Two rounds of narrowing, both driven by the ruling on [verl-project#26](verl-project#26) (2026-09-23) that new `PlatformBase` hooks are reserved for capabilities that genuinely can't be done from the plugin: 1. **`is_reduce_avg_supported()` dropped** — the xccl `ReduceOp.AVG` gap doesn't meet that bar. Its plugin-only replacement landed as [verl-project#32](verl-project#32) (merged): a monkeypatch over `torch.distributed.all_reduce`, scoped to xccl groups. 2. **`attention_utils_module()` and `dist_profiler_cls()` dropped** (2026-10-08, commit `439503c`), following the narrowed hook surface in the rework of [verl-project/verl#7917](verl-project/verl#7917): - `attention_utils_module()`: nothing is needed from the plugin. verl core routes every backend without a `flash_attn` build through its own pure-PyTorch `attention_padding_utils` (the former `npu_flash_attn_utils`, renamed device-neutral) — the same `bert_padding` port this hook pointed at. - `dist_profiler_cls()`: it added a second dispatch mechanism alongside core's hardcoded if/elif, so `VtuneProfiler` is now claimed by the `register_vtune.py` monkeypatch instead. The patch is double-gated — applied from `PlatformXPU.__init__`, not at import time, and behind a live `torch.xpu.is_available()` check, because verl's platform auto-detection instantiates every registered platform to probe it. - `profiler_markers()` is kept: core's module-level markers are imported by the trainer at module scope, so there is no plugin-side seam for them. `verl_hardware_plugin/accelerators/xpu/engines/fsdp_xpu.py` is untouched by this PR. ## Motivation Add in profiling of step annotation for intel gpu ## Changes - `verl_hardware_plugin/accelerators/xpu/platform_xpu.py`: add `profiler_markers()`; apply the VTune registration patch from `__init__` behind a live-device check. - `verl_hardware_plugin/accelerators/xpu/profilers/itt_profile_xpu.py` (new): ITT range markers (`mark_start_range`/`mark_end_range`/`mark_annotate`/`marked_timer`) and `VtuneProfiler(DistProfiler)`. - `verl_hardware_plugin/accelerators/xpu/profilers/register_vtune.py` (new): claims the `vtune` tool name by wrapping `DistProfiler.__init__`. - `verl_hardware_plugin/accelerators/xpu/profilers/__init__.py` (new): docstring note on registration. - `tests/test_plugin_registration.py` (new): hook + registration tests that run on a CPU-only host. - `docs/accelerators/xpu/{install_guidance,quick_start,profiling}.md` (new) and an updated `README.md`. ## verl-core dependency `profiler_markers()` only exists on verl-core builds that include #7917. Against stock `verl-project/verl:main` the plugin still installs and runs, but the hook has no base method to override, so ITT ranges aren't emitted; #7917 also lifts the `tool_config` allowlist in `engine_workers.py`, without which a `tool_config.vtune` entry is dropped. `VtuneProfiler` registration itself is pure monkeypatch and works either way. Use the pinned ref in `docker/intel_gpu/` for a known-good combination until #7917 merges. ## Testing - [x] `pytest tests/` — 199 passed on a CPU-only host (`torch 2.13.0+xpu`), including the new registration tests. Re-run after merging the accelerator reorganization (verl-project#41), which moved these modules to `accelerators/xpu/`. - [x] Smoke-checked on 1x Arc Pro B60 against a verl-core build *without* #7917, i.e. the degraded path described above: `PlatformXPU` selected (`xpu`/`intel`/`xccl`); ITT markers execute; `profiler.tool: vtune` gives `DistProfiler._impl = VtuneProfiler` with `start`/`step`/`stop` running, while `tool: torch` still resolves to core's own profiler; and with no #7917 the hook is absent, markers stay generic and the `tool_config` allowlist stays gated — a silent fallback, not an error. - [x] ITT emission is real on this stack, not a stub: `torch.profiler.itt.is_available()` is `True` and `torch._C._itt` exposes `rangePush`/`rangePop`/`mark`. **Not covered:** viewing the ranges in a VTune result. VTune attaches as an external collector, so `profiler_start`/`profiler_stop` are no-ops by design and there is no code path here that only a capture would exercise. It is also not runnable in the Intel GPU image: its oneAPI 2026.1 ships no `vtune` component and no `libittnotify`. Installing the collector is an image change, independent of this PR — written up in `docs/accelerators/xpu/profiling.md` (Prerequisites + Troubleshooting). ## Acceptance Baseline (for new hardware adaptation PRs) - [x] GRPO / PPO / GRPO+colocated-RM / SFT (`tests/special_intel_gpu/*.sh`) completed full training steps on B60 with this branch's platform changes live in the tree - [ ] `critic/rewards/mean` upward trend / NVIDIA-reference curve from `scripts/baseline_grpo_gsm8k.sh` specifically — **TODO: paste the SwanLab/log link, or re-run that script.** **SwanLab or training log link**: TODO ## Checklist - [x] Code follows the project's style and passes `pre-commit` checks - [x] Documentation updated - [x] No secrets or credentials included --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
Adds Intel VTune (ITT) profiling support for Intel XPU, plus the XPU user guide
that fills the stub
docs/accelerators/xpu/README.md.After the rework described below, this PR implements exactly one
PlatformBasehook —profiler_markers()— and does everything else from theplugin side:
PlatformXPU.profiler_markers()→ ITT range markers, so verl core can emittrace ranges without knowing about ITT or XPU.
VtuneProfiler(DistProfiler), selected byprofiler.tool: vtune, registeredby a plugin-side monkeypatch over
DistProfiler.__init__(
verl_hardware_plugin/accelerators/xpu/profilers/register_vtune.py) — no core hook.What changed since this PR was opened
Two rounds of narrowing, both driven by the ruling on
#26
(2026-09-23) that new
PlatformBasehooks are reserved for capabilities thatgenuinely can't be done from the plugin:
is_reduce_avg_supported()dropped — the xcclReduceOp.AVGgap doesn'tmeet that bar. Its plugin-only replacement landed as
#32 (merged):
a monkeypatch over
torch.distributed.all_reduce, scoped to xccl groups.attention_utils_module()anddist_profiler_cls()dropped (2026-10-08,commit
439503c), following the narrowed hook surface in the rework ofverl-project/verl#7917:
attention_utils_module(): nothing is needed from the plugin. verl coreroutes every backend without a
flash_attnbuild through its ownpure-PyTorch
attention_padding_utils(the formernpu_flash_attn_utils,renamed device-neutral) — the same
bert_paddingport this hook pointed at.dist_profiler_cls(): it added a second dispatch mechanism alongsidecore's hardcoded if/elif, so
VtuneProfileris now claimed by theregister_vtune.pymonkeypatch instead. The patch is double-gated —applied from
PlatformXPU.__init__, not at import time, and behind a livetorch.xpu.is_available()check, because verl's platform auto-detectioninstantiates every registered platform to probe it.
profiler_markers()is kept: core's module-level markers are imported bythe trainer at module scope, so there is no plugin-side seam for them.
verl_hardware_plugin/accelerators/xpu/engines/fsdp_xpu.pyis untouched by this PR.Motivation
Add in profiling of step annotation for intel gpu
Changes
verl_hardware_plugin/accelerators/xpu/platform_xpu.py: addprofiler_markers();apply the VTune registration patch from
__init__behind a live-device check.verl_hardware_plugin/accelerators/xpu/profilers/itt_profile_xpu.py(new): ITT range markers(
mark_start_range/mark_end_range/mark_annotate/marked_timer) andVtuneProfiler(DistProfiler).verl_hardware_plugin/accelerators/xpu/profilers/register_vtune.py(new): claims thevtunetool name by wrapping
DistProfiler.__init__.verl_hardware_plugin/accelerators/xpu/profilers/__init__.py(new): docstring noteon registration.
tests/test_plugin_registration.py(new): hook + registration tests that runon a CPU-only host.
docs/accelerators/xpu/{install_guidance,quick_start,profiling}.md(new) and anupdated
README.md.verl-core dependency
profiler_markers()only exists on verl-core builds that include #7917. Againststock
verl-project/verl:mainthe plugin still installs and runs, but the hookhas no base method to override, so ITT ranges aren't emitted; #7917 also lifts
the
tool_configallowlist inengine_workers.py, without which atool_config.vtuneentry is dropped.VtuneProfilerregistration itself ispure monkeypatch and works either way. Use the pinned ref in
docker/intel_gpu/for a known-good combination until #7917 merges.Testing
pytest tests/— 199 passed on a CPU-only host (torch 2.13.0+xpu),including the new registration tests. Re-run after merging the
accelerator reorganization (refactor: organize hardware plugin by accelerator #41), which moved these modules to
accelerators/xpu/.#7917, i.e. the degraded path described above:
PlatformXPUselected(
xpu/intel/xccl); ITT markers execute;profiler.tool: vtunegivesDistProfiler._impl = VtuneProfilerwithstart/step/stoprunning,while
tool: torchstill resolves to core's own profiler; and with no#7917 the hook is absent, markers stay generic and the
tool_configallowlist stays gated — a silent fallback, not an error.
torch.profiler.itt.is_available()isTrueandtorch._C._ittexposesrangePush/rangePop/mark.Not covered: viewing the ranges in a VTune result. VTune attaches as an
external collector, so
profiler_start/profiler_stopare no-ops by design andthere is no code path here that only a capture would exercise. It is also not
runnable in the Intel GPU image: its oneAPI 2026.1 ships no
vtunecomponent and no
libittnotify. Installing the collector is an image change,independent of this PR — written up in
docs/accelerators/xpu/profiling.md(Prerequisites + Troubleshooting).
Acceptance Baseline (for new hardware adaptation PRs)
tests/special_intel_gpu/*.sh)completed full training steps on B60 with this branch's platform changes
live in the tree
critic/rewards/meanupward trend / NVIDIA-reference curve fromscripts/baseline_grpo_gsm8k.shspecifically — TODO: paste theSwanLab/log link, or re-run that script.
SwanLab or training log link: TODO
Checklist
pre-commitchecks