-
Notifications
You must be signed in to change notification settings - Fork 556
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
base: master
Are you sure you want to change the base?
Updated build instructions #3127
Conversation
CONTRIBUTING.md
Outdated
```shell | ||
# Install build requirements | ||
conda env create -n openmm-dev -f devtools/ci/gh-actions/conda-envs/build-ubuntu-latest.yml |
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 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.
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.
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?
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 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.
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.
Providing an environment like this would also harmonize dependency installation across all supported platforms.
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.
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.
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.
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?
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.
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,
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.
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.
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?
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.
Also, maybe you have ideas about how to streamline the installation of a dev environment for the second case, @peastman?
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.
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!
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:
|
@peastman : Should we get these cleaned up and merged in before OpenMM 8 is released? |
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. |
Since Josh is no longer working with us, I'd suggest you try to rescue
whatever might be useful into a new PR.
|
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.