Skip to content

Deprecate sendnn_decoder in favor of sendnn with warmup_mode #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 5, 2025

Conversation

tjohnson31415
Copy link
Collaborator

If TORCH_SENDNN_LOG is set to WARNING instead of CRITICAL, there are logs stating

You're using a deprecated backend. Please use sendnn in conjunction with warmup_mode

This PR makes the change to sendnn. For backwards compatibility, sendnn_decoder is overwritten to sendnn and a warning is logged.

Copy link

👋 Hi! Thank you for contributing to vLLM support on Spyre.
Just a reminder: Make sure that your code passes all the linting checks, otherwise your PR won't be able to be merged. To do so, first install the linting requirements, then run format.sh and commit the changes. This can be done with uv directly:

uv sync --frozen --group lint --active --inexact

Or this can be done with pip:

uv pip compile --group lint > requirements-lint.txt
pip install -r requirements-lint.txt
bash format.sh

Now you are good to go 🚀

@tjohnson31415 tjohnson31415 force-pushed the warmup-mode branch 4 times, most recently from ae46c4a to e0bf459 Compare June 2, 2025 19:34
| Decoder | v0 | sendnn_decoder | V0 support for decoder models is deprecated |
| Decoder | v1 | sendnn_decoder | |
| Decoder | v0 | sendnn | V0 support for decoder models is deprecated |
| Decoder | v1 | sendnn | |
| Embedding | v0 | sendnn | |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I may have been mistaken when I wrote this and we actually need the sendnn_decoder backend for v0 embeddings. At least I think it looks like that's how our internal CI is set up to run them right now.

Would be good to double check that, though hopefully we deprecate soon anyway

Copy link
Collaborator Author

@tjohnson31415 tjohnson31415 Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my understanding is that decoder models can be used for embeddings as well (typically via "pooling").

I'm not following what needs to be checked. This PR is removing use of the sendnn_decoder backend (with a fallback to sendnn), so all embedding models would use sendnn.

@joerunde
Copy link
Collaborator

joerunde commented Jun 3, 2025

bot:test
MARKERS="embedding"

2 similar comments
@joerunde
Copy link
Collaborator

joerunde commented Jun 3, 2025

bot:test
MARKERS="embedding"

@joerunde
Copy link
Collaborator

joerunde commented Jun 3, 2025

bot:test
MARKERS="embedding"

@tjohnson31415
Copy link
Collaborator Author

bot:test
MARKERS="embedding"

@tjohnson31415
Copy link
Collaborator Author

A few internal CI tests are failing, but the same tests pass for me in my dev environment.

@joerunde
Copy link
Collaborator

joerunde commented Jun 4, 2025

I had to fix the internal automation, but I did see my last test run pass 🤔. Can't hurt to try again!

@joerunde
Copy link
Collaborator

joerunde commented Jun 4, 2025

bot:test
MARKERS="embedding"

@waleedqk
Copy link
Collaborator

waleedqk commented Jun 4, 2025

bot:test
MARKERS="embedding"

4 similar comments
@waleedqk
Copy link
Collaborator

waleedqk commented Jun 4, 2025

bot:test
MARKERS="embedding"

@waleedqk
Copy link
Collaborator

waleedqk commented Jun 4, 2025

bot:test
MARKERS="embedding"

@waleedqk
Copy link
Collaborator

waleedqk commented Jun 4, 2025

bot:test
MARKERS="embedding"

@waleedqk
Copy link
Collaborator

waleedqk commented Jun 4, 2025

bot:test
MARKERS="embedding"

@tjohnson31415 tjohnson31415 enabled auto-merge (squash) June 5, 2025 13:39
@github-actions github-actions bot added the ready label Jun 5, 2025
@tjohnson31415 tjohnson31415 merged commit 98dca8d into main Jun 5, 2025
21 checks passed
@tjohnson31415 tjohnson31415 deleted the warmup-mode branch June 5, 2025 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants