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

complete build.md #1508

Merged
merged 2 commits into from
Apr 30, 2024
Merged

complete build.md #1508

merged 2 commits into from
Apr 30, 2024

Conversation

YanxingLiu
Copy link
Contributor

Motivation

The earlier version of the build.md file lacked essential commands and prompts, leading to compilation errors. As a reuslt,
I've simply refined build.md.

Modification

docs/en/build.md
docs/zh-cn/build.md

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.

docs/en/build.md Outdated
@@ -55,6 +55,8 @@ Then, follow the steps below to set up the compilation environment:
```
- install [nccl](https://docs.nvidia.com/deeplearning/nccl/install-guide/index.html), and set environment variables:
```shell
git clone https://github.com/NVIDIA/nccl.git
Copy link
Collaborator

@lvhan028 lvhan028 Apr 29, 2024

Choose a reason for hiding this comment

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

I don't think building nccl from source is a good idea since there are prebuilt libraries.
Do you have any trouble installing nccl directly?
If so, we can discuss about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think building nccl from source is a good idea since there are prebuilt libraries. Do you have any trouble installing nccl directly? If so, we can discuss about it.

Directly install pre-compiled nccl is also is also possible, but previous environments NCCL_ROOT_DIR and NCCL_LIBRARIES seem to use the source-compiled nccl (since the pre-compiled nccl doesn't contain a build folder).
As a result, if I use pre-compiled nccl, the enviroments of nccl should be modified. I'll be submitting a version with pre-compiled nccl later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think building nccl from source is a good idea since there are prebuilt libraries. Do you have any trouble installing nccl directly? If so, we can discuss about it.

I have submitted a new version of document. Perhaps the author of the original documentation used the source compilation. So the environment variable needs to include the build folder, but pre-compiled nccl does not include the build folder. So simply removing build in NCCL_ROOT_DIR and NCCL_LIBRARIES will work!

Copy link
Contributor

Choose a reason for hiding this comment

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

So the environment variable needs to include the build folder, but pre-compiled nccl does not include the build folder. So simply removing build in NCCL_ROOT_DIR and NCCL_LIBRARIES will work!

ref to

# NCCL license: https://docs.nvidia.com/deeplearning/nccl/#licenses
mkdir tmp_nccl && cd tmp_nccl
wget -q https://developer.download.nvidia.com/compute/redist/nccl/v2.15.5/nccl_2.15.5-1+cuda11.8_x86_64.txz
tar xf nccl_2.15.5-1+cuda11.8_x86_64.txz
cp -a nccl_2.15.5-1+cuda11.8_x86_64/include/* /usr/local/cuda/include/
cp -a nccl_2.15.5-1+cuda11.8_x86_64/lib/* /usr/local/cuda/lib64/
cd ..
rm -rf tmp_nccl
ldconfig

@lvhan028 lvhan028 requested a review from AllentDan April 30, 2024 01:01
@lvhan028 lvhan028 added the documentation Improvements or additions to documentation label Apr 30, 2024
@lvhan028 lvhan028 merged commit 10f2c33 into InternLM:main Apr 30, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants