-
Notifications
You must be signed in to change notification settings - Fork 266
[Misc] Remove transformers from vllm-ascend requirements #1839
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
base: main
Are you sure you want to change the base?
Conversation
pip install -v -e . | ||
pip install -r requirements-dev.txt |
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.
why change this order
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.
for verify the the installation with vllm default transformers version without conflict
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.
I still don't understand why. In this PR. the transformers
has been removed from requirements. So install requirement
first won't change anything IMO.
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.
https://github.com/vllm-project/vllm-ascend/blob/33ef5dc8130fb7132b23c14d13a5c9361118d912/requirements-dev.txt#L16C1-L16C3 this will install transformers
, but for some users, installing requirements-dev
is not necessary
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.
but we should make sure requirements.txt
is installed before install the package.
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.
pip install -e .
will install req
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1839 +/- ##
=======================================
Coverage 54.18% 54.18%
=======================================
Files 74 74
Lines 9235 9235
=======================================
Hits 5004 5004
Misses 4231 4231
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: wangli <[email protected]>
Signed-off-by: wangli <[email protected]>
What this PR does / why we need it?
Remove transformers from vllm-ascend requirements, we do not need pin transformers to a lower version for this fix in the upstream
Does this PR introduce any user-facing change?
How was this patch tested?