-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Remove mamba-ssm package #22409
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
Remove mamba-ssm package #22409
Conversation
Signed-off-by: Tyler Michael Smith <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request removes the mamba-ssm
package and its related causal-conv-1d
dependency, which were only used for testing the PLaMo2 model against its Hugging Face implementation. The changes correctly remove the dependencies from requirements/test.in
, the Dockerfile, and documentation.
However, I've identified a critical issue in tests/models/language/generation/test_hybrid.py
where the PLaMo2 model (pfnet/plamo-2-1b
) is completely removed from the test suite, instead of just skipping the comparison with the Hugging Face baseline. I've provided a suggestion to fix this to ensure the model remains tested by vLLM.
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
Signed-off-by: Tyler Michael Smith <[email protected]>
Some other tests still require |
I see, you're right - there are both numeric issues and also seeing this:
|
As mentioned earlier #20047 (comment), some tests run pfnet/palmo-2-1b inference with transformers implementation, which implicitly depends on both |
Can't we just remove these as vLLM dependencies but install them inside the test image? This is what we are doing now for the causal_conv_1d package. Related topic: the hybrid tests are completely messed up right now due to some CUDA version mismatch issue. This would also be solved if we install mamba_ssm inside the test container but in the "right way" ( |
This is what I propose as a different solution to this problem: |
Closing in favor of #22541 |
Purpose
The packages
mamba-ssm
andcausal-conv-1d
are not used by vLLM but are used to compare against the huggingface transformer baselines in tests.In particular, those packages are need in order to run PLaMo2 at all. The model definition for PLaMo2 is at https://huggingface.co/pfnet/plamo-2-1b/blob/main/modeling_plamo.py, rather than being in huggingface transformers.
This PR removes the packages to avoid pain points when upgrading PyTorch or CUDA.
Test Plan
Test Result