Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Comments

Close side panel

Fix to vllm compile error#2686

Closed
deewhyweb wants to merge 5 commits intoinstructlab:maininstructlab/instructlab:mainfrom
deewhyweb:maindeewhyweb/instructlab:mainCopy head branch name to clipboard
Closed

Fix to vllm compile error#2686
deewhyweb wants to merge 5 commits intoinstructlab:maininstructlab/instructlab:mainfrom
deewhyweb:maindeewhyweb/instructlab:mainCopy head branch name to clipboard

Conversation

@deewhyweb
Copy link

@deewhyweb deewhyweb commented Nov 21, 2024

podman build of the cuda containerfile is failing because of

ImportError: libcudnn.so.9: cannot open shared object file: No such file or directory

This adds the libcudnn9 libraries to resolve the issue

@mergify mergify bot added container Affects containization aspects ci-failure PR has at least one CI failure labels Nov 21, 2024
@nathan-weinberg
Copy link
Member

nathan-weinberg commented Nov 21, 2024

@deewhyweb thanks for the contribution! can you sign off your commit please?

@deewhyweb
Copy link
Author

deewhyweb commented Nov 21, 2024

Added signoff

@nathan-weinberg
Copy link
Member

nathan-weinberg commented Nov 21, 2024

Needs to be squashed into one

@deewhyweb
Fix to vllm compile error
48068fb 
Update to PR

Signed-off-by: Philip Hayes <philip@deewhy.ie>
@deewhyweb
Copy link
Author

deewhyweb commented Nov 21, 2024

Squashed

@mergify mergify bot removed the ci-failure PR has at least one CI failure label Nov 21, 2024
nathan-weinberg
nathan-weinberg reviewed Nov 21, 2024
View reviewed changes
Copy link
Member

@nathan-weinberg nathan-weinberg left a comment

Choose a reason for hiding this comment

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

Small nit but otherwise LGTM

containers/cuda/Containerfile Outdated Show resolved Hide resolved
@deewhyweb @nathan-weinberg
Update containers/cuda/Containerfile
7da4d74 
Co-authored-by: Nathan Weinberg <31703736+nathan-weinberg@users.noreply.github.com>
Signed-off-by: Philip Hayes <philip@deewhy.ie>
@mergify mergify bot added one-approval PR has one approval from a maintainer ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Nov 22, 2024
@nathan-weinberg
Copy link
Member

nathan-weinberg commented Nov 22, 2024

@deewhyweb @pacificera @clopnis would this fix close #2209?

@nathan-weinberg nathan-weinberg requested a review from a team November 22, 2024 19:39
@mergify mergify bot added the ci-failure PR has at least one CI failure label Nov 22, 2024
@cdoern
Copy link
Contributor

cdoern commented Nov 22, 2024

This change looks fine to me, unsure why it is failing here

@mergify mergify bot removed the ci-failure PR has at least one CI failure label Nov 26, 2024
@deewhyweb
Change to vllm tag to fix sdg issue
9769656 
Signed-off-by: Philip <philip@deewhy.ie>
@deewhyweb
Copy link
Author

deewhyweb commented Nov 26, 2024

@nathan-weinberg @cdoern Updated PR to downgrade vllm tag to v0.6.1 to resolve SDG error resulting in:

WARNING 2024-11-21 20:54:57,568 instructlab.sdg.generate_data:445: Empty dataset for qna node: knowledge_deloreon
INFO 2024-11-21 20:54:57,569 instructlab.sdg.generate_data:485: Generation took 89.70s
WARNING 2024-11-21 20:54:57,569 instructlab.sdg.generate_data:487: Leaf nodes with empty sdg output: knowledge_deloreon

@deewhyweb
Update to requirements for vllm-cuda
bd3387d 
Signed-off-by: Philip <philip@deewhy.ie>
@mergify mergify bot added dependencies Relates to dependencies ci-failure PR has at least one CI failure labels Nov 26, 2024
@nathan-weinberg
Copy link
Member

nathan-weinberg commented Jan 10, 2025

We will likely have to rebase this after #2892 merges, but I'd still be interested in getting this in!

@courtneypacheco
Copy link
Contributor

courtneypacheco commented Mar 6, 2025

Hey @deewhyweb ! I'm looking to do some housekeeping and close out stale PRs, including drafts.

This PR appears to be stale because I haven't seen any recent activity, but I also noticed that @nathan-weinberg wanted this PR merged after #2892 was merged. (It's now merged.)

@nathan-weinberg, Do we still need this PR?

@courtneypacheco
Copy link
Contributor

courtneypacheco commented Mar 17, 2025

Hi @deewhyweb. I'm going to close this PR due to inactivity. However, feel free to reopen it and rebase against main if you feel this change is still needed. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@n1hility n1hility Awaiting requested review from n1hility

1 more reviewer

@nathan-weinberg nathan-weinberg nathan-weinberg approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ci-failure PR has at least one CI failure container Affects containization aspects dependencies Relates to dependencies one-approval PR has one approval from a maintainer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@deewhyweb @nathan-weinberg @cdoern @courtneypacheco
Morty Proxy This is a proxified and sanitized view of the page, visit original site.