vulkan: allow proxy collectives without external semaphore FD functions - #23
Open
AndrewTilson wants to merge 1 commit into
Open
AndrewTilson wants to merge 1 commit into
AndrewTilson wants to merge 1 commit into
Conversation
Prepared with AI assistance for personal-fork review; not an upstream submission.
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.
Would you be willing to merge this into your branch so if/when you pr makes it to llama repo it incudes these minor changes. As you can see below there was some significant gains from this minor change on my windows system. I am running dual rtx 9700's.
Scope
Allow the existing portable CPU-proxy collective path to initialize when Linux
external semaphore FD functions are unavailable. Host-importable memory remains
required. Missing FD functions select the proxy rather than disabling the fast
collective implementation; the explicit proxy option preserves that selection.
One source file, four additions and three deletions. No harness changes, tuning
switches, diagnostic logging, or experimental Win32 import code are included.
Dependency and attribution
This PR deliberately targets
review/pr25051-base, not main, so it shows onlyour incremental fix to the proposed allreduce implementation:
13ac9015672d4e934d97cd19dd46f3ede4536a41.fix assertion DFlash2 with
--split-mode tensoron CUDA ggml-org/llama.cpp#278583286b033c5dc8bcce9958f724d4e4a93ae589646.The allreduce and DFlash implementations belong to their respective authors.
The tested combined tree also needed port adaptations; those are not part of
this small patch. Do not merge this base into main as if it were our own work.
Historical measurements and limits
Two R9700s, Windows Vulkan, driver 32.0.23033.1002, Q4_K_S target and Q4 DFlash
Combined integration | Passing requests | Aggregate output tokens/s -- | -- | -- Before proxy initialization fix | 36/36 | 33.891 With proxy initialization fix | 36/36 | 38.797 Same-period single-card control | 12/12 | 55.703 Same-period dual-layer control | 12/12 | 44.395drafter, Q8 K/V, flash attention, 32K context, one slot, concurrency one.
Short JSON/code/tool fixtures, same workload across comparisons:
These are historical combined-build results, NOT a hardware benchmark of this
exact isolated patch on its PR25051-only base. Later post-reboot improvements
are not attributed to this fix. Tensor still did not beat single-card.
Short syntax/JSON/tool checks do not establish broad quality or correctness.
Direct TG16 diagnostic (one repetition, different settings from API tests):
17.235 before versus 19.540 tokens/s after; trace changed from fast=0 to
fast=1/proxy=1. No universal speedup is asserted.
Validation
git diff --checkpasses.The fix is backend-level, not Python-harness-specific, but affects only
Scope Allow the existing portable CPU-proxy collective path to initialize when Linux external semaphore FD functions are unavailable. Host-importable memory remains required. Missing FD functions select the proxy rather than disabling the fast collective implementation; the explicit proxy option preserves that selection.consumers using this proposed Vulkan meta-backend collective implementation.
Coordinate with the upstream PR author before any upstream submission.
One source file, four additions and three deletions. No harness changes, tuning
switches, diagnostic logging, or experimental Win32 import code are included.
Dependency and attribution
This PR deliberately targets review/pr25051-base, not main, so it shows only
our incremental fix to the proposed allreduce implementation:
ggml-org#25051
Base: 13ac901.
Historical DFlash integration tests also used
ggml-org#27858
Revision: 3286b03.
Related startup issue: ggml-org#27833.
The allreduce and DFlash implementations belong to their respective authors.
The tested combined tree also needed port adaptations; those are not part of
this small patch. Do not merge this base into main as if it were our own work.
Historical measurements and limits
Two R9700s, Windows Vulkan, driver 32.0.23033.1002, Q4_K_S target and Q4 DFlash
drafter, Q8 K/V, flash attention, 32K context, one slot, concurrency one.
Short JSON/code/tool fixtures, same workload across comparisons:
Combined integration Passing requests Aggregate output tokens/s
Before proxy initialization fix 36/36 33.891
With proxy initialization fix 36/36 38.797
Same-period single-card control 12/12 55.703
Same-period dual-layer control 12/12 44.395
These are historical combined-build results, NOT a hardware benchmark of this
exact isolated patch on its PR25051-only base. Later post-reboot improvements
are not attributed to this fix. Tensor still did not beat single-card.
Short syntax/JSON/tool checks do not establish broad quality or correctness.
Direct TG16 diagnostic (one repetition, different settings from API tests):
17.235 before versus 19.540 tokens/s after; trace changed from fast=0 to
fast=1/proxy=1. No universal speedup is asserted.
Validation
Patch applies to the exact pinned base.
Applied contents match expected extraction; reverse apply check passes.
git diff --check passes.
Clean isolated patch build and matched hardware benchmark.
Regression test for capability/transport selection.
Backend-op correctness and perplexity validation.
Linux native-FD regression and additional Windows workload coverage.
The fix is backend-level, not Python-harness-specific, but affects only
consumers using this proposed Vulkan meta-backend collective implementation.
Coordinate with the upstream PR author before any upstream submission.