Add Metal/MPS GPU backend for the ungapped prefilter on Apple Silicon#1121
Open
fnachon wants to merge 3 commits into
Open
Add Metal/MPS GPU backend for the ungapped prefilter on Apple Silicon#1121fnachon wants to merge 3 commits into
fnachon wants to merge 3 commits into
Conversation
Implements a Metal-based counterpart to lib/libmarv (CUDA/HIP) behind a new ENABLE_MPS CMake option, so --gpu 1 works on Apple Silicon Macs. Scope is the default ungapped prefilter (GAPLESS) only; the gapped Smith-Waterman GPU path remains CUDA/HIP-only. - lib/libmarv-mps/: Metal implementation of the Marv interface (marv.h), reusing it unchanged so GpuUtil/gpuserver/ungappedprefilter need no backend-specific logic. Kernel design: one simdgroup cooperates per alignment (registers-in-threadgroup-memory DP state, coalesced PSSM layout), with work-stealing dispatch across a shared atomic counter so skewed sequence-length databases don't stall on the longest outliers. submitScan()/collectScan() split lets the query loop overlap GPU execution with building the next query's profile. - marv.h: additive submitScan()/collectScan() declarations for the async split; CUDA/HIP is unaffected since nothing calls them under that build. - CMakeLists.txt, src/CMakeLists.txt: ENABLE_MPS option, mutually exclusive with ENABLE_CUDA. - GpuUtil.cpp, Parameters.cpp, gpuserver.cpp, ungappedprefilter.cpp: extend existing HAVE_CUDA guards to HAVE_MPS; CUDA/HIP code paths are unchanged. - README.md: build/usage note for the new backend. Validated on an Apple M5 Max: byte-identical results against a scalar CPU reference and against MMseqs2's own CPU prefilter (modulo the CPU path's intentional 8-bit score saturation), across synthetic boundary-length cases and real searches up to 153.7M sequences (UniRef90). GPU search against UniRef90 measured ~8.5x faster than the CPU path's k-mer prefilter for the same query on this database size. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
|
Interesting, i was playing with this recently too: |
pipeline/prevQueryKey/prevQueryLen/havePending were declared unconditionally (with pipeline forced to false via an #else branch for non-MPS builds), but only ever referenced from code gated behind #ifdef HAVE_MPS. Under a CUDA/HIP build, that left them declared-but-unused, which upstream's CI build (-Wall -Wextra -Werror) correctly rejects. Move all four declarations inside the same #ifdef HAVE_MPS block as their uses, so they don't exist at all in a CUDA/HIP translation unit. Verified on real hardware (2x RTX A6000, CUDA 13.3, gcc 12.2), not just inferred: built from scratch with -DENABLE_CUDA=1 on the pre-fix commit to reproduce the exact CI warnings, then rebuilt after this fix with the CI's literal compile flags (-Wall -Wextra -Wdisabled-optimization -Werror -pedantic -fno-exceptions) -- clean compile, full link, working binary. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Subject bytes were folded via `& 31` to collapse MMseqs2's soft-masked
(lowercase, base+32) residue encoding back to the unmasked base residue,
modeled on convert.cuh's CaseSensitive_to_CaseInsensitive from libmarv's
CUDA code. That turned out to be the wrong model for this kernel: the CPU
prefilter path (StripedSmithWaterman.cpp) and libmarv's real CUDA kernels
both treat masked/out-of-alphabet residues as suppression -- clamped to
the last alphabet index ("X") -- not as a reversible case encoding to fold
away. Folding instead of clamping silently un-masks masked positions,
scoring them at full value instead of down-weighting them, defeating the
purpose of masking (suppressing spurious hits to low-complexity/repeat
regions).
Fix: clamp out-of-alphabet bytes to alphabetSize-1 once in loadDb(), when
the database is loaded, rather than folding per-residue in the kernel's
hot loop -- cheaper (one pass over the DB instead of every kernel access)
and matches the CPU path's semantics exactly.
Found by direct comparison against an independent Metal implementation
(github.com/milot-mirdita/mmseqs2, branch metal-gpu-backend) that scores
real database sequences correctly. On a real query/target pair with masked
residues, this bug had been inflating the ungapped score by ~15% (7124 vs.
the correct 6163, independently confirmed via a from-scratch Python DP
reference and cross-checked byte-for-byte identical PSSM/subject input
data between both implementations) -- not a rare edge case, since any
subject with soft-masked residues was affected, worst for near-duplicate/
self-hits and low-complexity regions. Re-verified against the full
boundary-length suite and the 500-query/20k-sequence benchmark; cutoff
scores and ~95% of top-300 hit sets now match the independent
implementation exactly (remaining differences are the same benign
tie-break-at-cutoff pattern already documented elsewhere in this file).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Author
|
Thanks for the pointer, @milot-mirdita — A clearly more sophisticated implementation! After the fix, I re-ran a real query (a protein I work with, ~290 aa) against the full UniRef90 (153.7M sequences) on both implementations:
Given that, it makes more sense for the project to converge on your approach! |
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 a Metal-based counterpart to
lib/libmarv(the CUDA/HIP GPU backend), so--gpu 1works on Apple Silicon Macs, behind a newENABLE_MPSCMake option (mutually exclusive withENABLE_CUDA).Scope: accelerates the default ungapped prefilter (
AlignmentType::GAPLESS, i.e.--prefilter-mode 0, the same mode--gpu 1uses on CUDA today). The gapped Smith-Waterman GPU kernel (--prefilter-mode 2) is not ported and remains CUDA/HIP-only; the constructor throws a clear error if that mode is requested on this backend.Design
lib/libmarv-mps/: a Metal implementation of theMarvclass declared in the existinglib/libmarv/src/marv.h, reused unchanged soGpuUtil.cpp/gpuserver.cpp/ungappedprefilter.cppneed no backend-specific branching beyondHAVE_MPScompile guards mirroring the existingHAVE_CUDAones.threadgroupmemory, with a singlesimd_shuffle_upper subject residue carrying the one cross-lane dependency the recurrence has. The PSSM is uploaded pre-transposed for coalesced access. Work-stealing dispatch (a shared atomic counter) means simdgroups pull the next database entry as they finish, rather than a static one-simdgroup-per-sequence assignment — this matters a lot on real, length-skewed protein databases where a handful of long sequences would otherwise dominate a naive dispatch's wall-clock while short-sequence threads sit idle.marv.hgains two new, additive public methods,submitScan()/collectScan()— an optional async 2-phase form ofscan()that lets the query loop inungappedprefilter.cppoverlap GPU execution of query i with building query i+1's profile on the CPU. Only the Metal backend implements them; CUDA/HIP is unaffected since nothing calls them underHAVE_CUDAbuilds.marv_mps.mm) — kept as documented history rather than shipped, since it regressed performance.Validation
Tested on an Apple M5 Max:
Build
Usage is identical to the existing CUDA GPU workflow (
makepaddedseqdb+--gpu 1).Test plan
-DENABLE_MPS=1-DENABLE_CUDA=1/ default CPU-only configure is unaffected (byte-identical CMake behavior, noHAVE_MPS-guarded code compiled)HAVE_CUDA-guarded paths are untouched (I don't have NVIDIA/AMD hardware to test this myself — the diffs there are mechanical macro-condition extensions, but worth a maintainer's eyes)🤖 Generated with Claude Code