8000 Updated build instructions by Yoshanuikabundi · Pull Request #3127 · openmm/openmm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Updated build instructions #3127

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Yoshanuikabundi
Copy link
Member
@Yoshanuikabundi Yoshanuikabundi commented May 27, 2021

Previously, the build instructions especially for documentation was out of date and was difficult to locate. With this PR, simple instructions for experienced developers are found in CONTRIBUTING.md, the reader is referred to the appropriate section of the User Guide from both CONTRIBUTING.md and the introduction of the Developer's Guide, and the User Guide has been updated to be more complete and accurate.

Closes #3125 and closes #3126.

@Yoshanuikabundi
Copy link
Member Author

@peastman I have removed references to the bug you fixed in #3129. There are a lot of other important corrections and additions in this PR.

CONTRIBUTING.md Outdated
```shell
# Install build requirements
conda env create -n openmm-dev -f devtools/ci/gh-actions/conda-envs/build-ubuntu-latest.yml
Copy link
Member

Choose a reason for hiding this comment

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

I looked over these files, and I don't think we should direct users to them. They install a lot of things that are used in CI but are irrelevant to most users (Gromacs, ccache, ocl-icd, etc.). I think the following command will install everything they need from conda (but please check it to be sure!):

conda install -c conda-forge cmake cython swig fftw doxygen=1.8.14

The doxygen pin might not be necessary. It's in the yaml file, but newer versions might work.

This leaves out an essential piece, of course: a compiler. Anyone doing development probably already has that, but we should direct them to the instructions in the manual. Building documentation requires additional packages, which are different depending on whether you're building HTML or PDF documentation. That too is probably best to just direct them to the manual for.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, there's another critical piece that's being left out here: CUDA and/or OpenCL. That involves both an SDK and a driver, and depends on what kind of GPU you have.

The more I think about it, the less sure I am that trying to give minimal build instructions here is a good idea. Maybe it would be better to just link to the manual and leave it at that?

Copy link
Member

Choose a reason for hiding this comment

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

I think the following command will install everything they need from conda (but please check it to be sure!)
conda install -c conda-forge cmake cython swig fftw doxygen=1.8.14

The whole point of making it easy for users to install development environments is to avoid having them manually install a bunch of dependencies. At the very least, we should provide a current conda YAML environment that can be used to create a working build environment. Even better would be to also push this environment to conda-forge and keep it current so you can simply specify the build environment to be installed from conda-forge, like qcfractal does.

Copy link
Member

Choose a reason for hiding this comment

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

Providing an environment like this would also harmonize dependency installation across all supported platforms.

Copy link
Member

Choose a reason for hiding this comment

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

We want to make it as easy as possible for people to set up a useful development environment. If we somehow update the manual to have the simplest, clearest achievable way of doing this, we can just point to it. But the current installation instructions is a loooooong way from this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please see the latest commit in this PR, where the proposed environment file is implemented and CONTRIBUTING.md has been updated to provide instructions on its use.

You also need to install the CUDA and/or OpenCL SDK appropriate for your GPU. Those are not conda installable.

Actually they are; see the feedstock where the conda distributed binaries are built from conda dependencies only. The trick is that there isn't a one size fits all solution, different hardware needs different packages. In the latest commit to this PR, I explicitly discuss this, and note that the environment is only sufficient to build the CPU platform. I think this will cover a lot of development needs, and I also provide hints for finding the appropriate GPU sdk package for contributors who need it.

The "cudatoolkit" package is not really the full toolkit, only the redistributable libraries.

cudatoolkit-dev includes the headers.

Also, this procedure will often produce libraries that can only be used within the conda environment. That's because conda-forge ships its own versions for lots of system libraries. If someone is building libraries to use from a C++ or Fortran program, there's a good chance they'll incompatible libraries that don't work with the standard system libraries built into the OS.

True, but this is a feature, not a bug, as it allows devs to maintain the entire dependency chain in a single conda environment rather than having libraries strewn across their system. I'd argue that this is actually the entire point of using conda!

I think we really need to document two separate procedures, depending on whether you're building packages to use inside or outside a conda environment.

This PR doesn't remove any documentation. The existing build guide in the manual still exists. I can add a line to contributing.md mentioning this caveat if you like?

Copy link
Member

Choose a reason for hiding this comment

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

Please see the latest commit in this PR

That's looking a lot better! Just a few more comments.

cudatoolkit-dev includes the headers.

Even that still isn't sufficient to build it, unfortunately. To build the conda packages, we need to use special Docker images that have the necessary (non-redistributable) components pre-installed in them. For example,

https://github.com/conda-forge/openmm-feedstock/blob/master/.azure-pipelines/azure-pipelines-linux.yml#L14

I don't know of any way to get a complete, functional CUDA build environment that doesn't involve obtaining packages directly from NVIDIA.

True, but this is a feature, not a bug

That depends on what you're doing! :) If you're a Python developer building a library to use inside a conda environment, it's a feature. If you're a C++ developer building a library to link with a stand-alone application you're writing, it's a bug.

Copy link
Member

Choose a reason for hiding this comment

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

It seems like we're dealing with two use cases here, then. Why not split this into two paths, especially since the first (Python) path is likely to be quite popular and very easy/streamlined?

Copy link
Member

Choose a reason for hiding this comment

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

Also, maybe you have ideas about how to streamline the installation of a dev environment for the second case, @peastman?

Copy link
Member Author

Choose a reason for hiding this comment

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

The paths wouldn't be Python/Not Python, they'd be GPU platforms/everything else. But yes! Different use cases.

I don't know of any way to get a complete, functional CUDA build environment that doesn't involve obtaining packages directly from NVIDIA.

cudatoolkit-dev just pulls the SDK installer down from NVIDIA and executes it. I'm installing it now, so I guess we'll find out if it works shortly!

@Yoshanuikabundi
Copy link
Member Author

In our check-in meeting today, @peastman and I resolved to postpone this PR until we have a bigger picture of what users and contributors want. In particular:

  • How valuable is a self-contained conda environment? Distributing the compiler (see Document how to set up Python wrapper in editable/dev mode #3156) and GPU build dependencies are problematic (but theoretically possible), generally because conda-forge optimises these for use with conda-build, not for end users. Using system compilers and GPU SDKs may be worth the additional step, but we need to ensure that my linking difficulties are solvable.
  • Are native Windows instructions valuable, or would a suggestion to use WSL suffice?

@jchodera
Copy link
Member
jchodera commented Jul 3, 2022

@peastman : Should we get these cleaned up and merged in before OpenMM 8 is released?

@peastman
Copy link
Member
peastman commented Jul 4, 2022

I don't think we ever reached a clear picture of what it should be. On the other hand, this PR also includes a bunch of minor changes to other files. It would be good to copy them to a new PR so we can merge them. I can do that if you want.

@jchodera
Copy link
Member
jchodera commented Jul 4, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation instructions are difficult to find Documentation build instructions are incorrect
3 participants
0