Skip to content
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

fix: build from source with MPI & NCCL in other directories #471

Closed
wants to merge 1 commit into from

Conversation

C1rN09
Copy link
Contributor

@C1rN09 C1rN09 commented Sep 25, 2023

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily receiving feedbacks. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

Tried building lmdeploy from source with NCCL & MPI installed in other directory, here is my commands

mkdir build && cd build
NCCL_ROOT=/my/path/to/nccl bash ../generate.sh

This gets me wrong because

  1. CMAKE_POLICY 0074 not enabled, so NCCL_ROOT doesn't work
  2. MPI library path is hardcoded so mine was not found

Modification

  1. Enable CMP0074
  2. fix NCCL & MPI related paths

BC-breaking (Optional)

No

Use cases (Optional)

If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit tests to ensure the correctness.
  3. If the modification has a dependency on downstream projects of a newer version, this PR should be tested with all supported versions of downstream projects.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

@lvhan028 lvhan028 requested a review from lzhangzz September 26, 2023 06:12
@lvhan028 lvhan028 self-requested a review October 9, 2023 03:00
@C1rN09 C1rN09 closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants