[Core] Force PIECEWISE CUDAGraph mode for encoder-decoder #25701
Conversation
russellb
requested review from
ProExpertProg,
WoosukKwon,
hmellor,
houseroad,
mgoin,
robertgshaw2-redhat,
simon-mo,
tlrmchlsmth,
yewentao256 and
youkaichao
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to disable full CUDA graphs for encoder-decoder models, such as Whisper, by forcing them into PIECEWISE mode. While the intent is correct, the implementation contains a critical logical error. The condition self.model_config.is_encoder_decoder is not None will always evaluate to True because the is_encoder_decoder property returns a boolean. This would cause all models to incorrectly fall back to PIECEWISE mode, potentially leading to performance regressions. I have provided a review comment with a suggested fix to correct this condition.
Whisper does not work with full cudagraphs. That is being worked on in PR vllm-project#25208. The failure can be reproduced reliably via `tests/models/multimodal/generation/test_whisper.py`, at least in my H100 development environment. The tests passed on the PR and I'm not sure why. Regardless, this seems like the right change to make until vllm-project#25208 sorts out exactly what changes are needed. Signed-off-by: Russell Bryant <rbryant@redhat.com>
russellb
force-pushed
the
whisper-cudagraphs-piecewise
branch
from
a36468b to
3e50d7f
Compare
ProExpertProg
added
the
ready
We do have a test that does pretty extensive accuracy testing. It tests a big dataset and measures the word error rate of our transcription. It will fail if the error rate goes above a certain threshold. It's in |
no big deal. I'm just confused why it works in CI but not locally. Is there something about the older GPUs in CI that would turn off cudagraphs or something? |
Signed-off-by: Russell Bryant <rbryant@redhat.com> Signed-off-by: yewentao256 <zhyanwentao@126.com>
…ct#25701) Signed-off-by: Russell Bryant <rbryant@redhat.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
…ct#25701) Signed-off-by: Russell Bryant <rbryant@redhat.com>
…ct#25701) Signed-off-by: Russell Bryant <rbryant@redhat.com>
…ct#25701) Signed-off-by: Russell Bryant <rbryant@redhat.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
6 participants
Whisper does not work with full cudagraphs. That is being worked on in
PR #25208.
The failure can be reproduced reliably via
tests/models/multimodal/generation/test_whisper.py, at least in myH100 development environment. The tests passed on the PR and I'm not
sure why.
Regardless, this seems like the right change to make until #25208
sorts out exactly what changes are needed.
Signed-off-by: Russell Bryant rbryant@redhat.com