-
Notifications
You must be signed in to change notification settings - Fork 112
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
Update Build Instructions #913
base: master
Are you sure you want to change the base?
Conversation
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.
Good idea to refactor this, some ideas from my side.
IMHO, we have to be careful with $PREFIX
. The most important part is that it is consistent everywhere.
----------------------------- | ||
|
||
Compiling SeisSol | ||
~~~~~~~~~~~~~~~~~ | ||
Get the latest version of SeisSol on git by cloning the whole repository | ||
Get the latest version of SeisSol on git by cloning the whole repository, |
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.
Should we add something like: "Get the latest release (https://github.com/SeisSol/SeisSol/releases), by cloning the whole repository: git clone --recursive --branch vX.Y.Z https://github.com/SeisSol/SeisSol.git
"
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'll add it. Maybe also a note that the master branch may be unstable? (but contains the latest features)
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.
Yes, that's a good idea.
mkdir build-release && cd build-release | ||
CC=mpicc CXX=mpiCC FC=mpif90 cmake -DCOMMTHREAD=ON -DNUMA_AWARE_PINNING=ON -DASAGI=ON -DCMAKE_BUILD_TYPE=Release -DHOST_ARCH=skx -DPRECISION=double -DORDER=4 -DGEMM_TOOLS_LIST=LIBXSMM,PSpaMM .. | ||
mkdir build && cd build | ||
cmake -DCMAKE_BUILD_TYPE=Release -DHOST_ARCH=hsw -DPRECISION=double -DORDER=4 -DGEMM_TOOLS_LIST=Eigen -DEQUATIONS=elastic -DGRAPH_PARTITIONING_LIBS=none .. |
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.
Given the fact that a fair amount of people will just copy-paste this line, should we use hsw
, libxsmm
, parmetis
as default?
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, why not GEMM_TOOLS_LIST=auto
?
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.
GEMM_TOOLS_LIST=auto
, to give an example, will assume libxsmm
on Haswell, regardless if it's installed or not. Maybe it's worth checking the GEMM_TOOLS_LIST
entries before adding them when using auto
. Maybe there'll be some PR for that soon-ish—but not now, unfortunately; therefore I'd recommend Eigen
, as we'll not have to consider any other dependency. Maybe we'll have to split into a "minimal" and a "optimal"(?) build.
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'd vote for simplicity, which is to provide one set of instructions, which works for almost everybody with good performance.
Installing libxsmm is easy, every user should do it sooner or later anyways. If they've compiled netcdf/hdf5 successfully, they will also easily install libxsmm. Thus, I don't see a reason for distinguishing between minimal/optimal, but I would just give a default.
The following parameters are important to note: | ||
|
||
``PRECISION`` is either ``single`` or ``double`` and corresponds to 32-bit floating point numbers and 64-bit floating point numbers | ||
used during all calculations. If your simulation gives an error when using single precision, it can sometimes be fixed by re-compiling for double precision. |
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.
If you experience Inf/Nan errors, when using single precision...
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 would also write:
see also #200
The ``ORDER`` parameter may range from 2 to 8. The ``EQUATIONS`` parameter may be ``elastic``, ``viscoelastic2``, ``poroelastic``, | ||
or ``anisotropic``. For the ``viscoelastic2`` setting, you will need to add e.g. ``-DNUMBER_OF_MECHANISMS=3`` (or replace 3 by any other number greater than zero). | ||
|
||
Once you have installed a GEMM generator, choose a different option in ``GEMM_TOOLS_LIST`` to obtain a faster version of SeisSol. |
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.
Would change this to:
The GEMM generator is used to create optimized code for matrix-matrix multiplications.
* `libxsmm` should give reasonable performance on most x86 CPUs.
* For the `skx` architecture, we suggest `libxsmm,PSPaMM`.
* If you have trouble with these libraries, you can use `Eigen`, which should work on most available architectures.
- IBM PowerPC 9 | ||
- | ||
|
||
In case of a manual installation of dependencies (and not using the `setup.sh` script from the "Installing Dependencies" page), you may have to prepend :code:`CMAKE_PREFIX_PATH` and :code:`PKG_CONFIG_PATH` to the cmake command, e.g. for dependencies installed in :code:`${PREFIX}`: |
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 is important that all dependencies and SeisSol are compiled with the same compiler. In particular, gcc
and intel
compilers are not compatible.
On some clusters, where several compilers are installed, you might want to specifically point to the compiler, which you want to use for SeisSol. For the intel
compiler use e.g. CC=mpiicc CXX=mpiicpc cmake ...
.
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.
gcc/clang however are typically not a problem
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.
Maybe: In particular, we have experienced problems when mixing gcc/clang with intel compilers
.
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 don't particularly need to use mpicc
/mpiicc
(and mpicxx
/mpiicpc
), do we? I.e. the non-MPI compilers should be good... Or are they?
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.
In my experience, using the non-mpi compilers, typically cmake can't find the proper libraries. That's why I always use the mpi wrappers.
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've tested the non-mpi compilers (on a gcc12 openmpi compilation) and it was working fine.
You will also need CMake in version 3.20.0 or above. Most likely, you system will already have a | ||
version of CMake installed; however, you may have to load a module to get a new enough version. | ||
|
||
If you do not have CMake in a new enough version available, you may also install it manually as follows. | ||
|
||
.. code-block:: bash | ||
|
||
# you will need at least version 3.20.0 for GNU Compiler Collection | ||
(cd $(mktemp -d) && wget -qO- https://github.com/Kitware/CMake/releases/download/v3.20.0/cmake-3.20.0-Linux-x86_64.tar.gz | tar -xvz -C "." && mv "./cmake-3.20.0-linux-x86_64" "${HOME}/bin/cmake") |
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's ${HOME}/bin/make
here, but ${PREFIX}/bin/...
below
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.
Thanks! Missed that one. Maybe PREFIX
should be SEISSOL_PREFIX
instead, that would make it more easy to recognize. The choice for PREFIX
was mainly since if some make
script has a prefix variable, it's sometimes named PREFIX
, i.e. setting the environment variable in advance will spare us from setting that explicitly again in this case. ... But we actually don't really run into that, so... I'll rename it.
~~~~~~~~~~~~~~~ | ||
""""""""""""""" | ||
|
||
We begin with HDF5 which is needed for reading meshes in PUML format (the default format in SeisSol, and the output of PUMgen). |
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.
HDF5, which
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 would remove "We begin with".
HDF5 is needed ...
- PSpaMM (pspamm.py) for small sparse matrix multiplications (required only on Knights Landing or Skylake) | ||
|
||
Note that using Eigen does not result in any additional dependencies, since it is needed in SeisSol anyways. | ||
Other than that, we recommend using the combination libxsmm and PSpaMM. |
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 recommend libxsmm
and in addition PSpaMM
for architectures with AVX-512
.
wget https://syncandshare.lrz.de/dl/fiJNAokgbe2vNU66Ru17DAjT/netcdf-4.6.1.tar.gz | ||
tar -xaf netcdf-4.6.1.tar.gz | ||
cd netcdf-4.6.1 | ||
CFLAGS="-fPIC ${CFLAGS}" CC=h5pcc ./configure --enable-shared=no --prefix=$HOME --disable-dap |
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.
prefix=$PREFIX
?!
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.
See the comment above, it's actually correct here. But I agree that it might seem confusing.
ASAGI | ||
""""" | ||
|
||
See section :ref:`Installing SYCL <installing_SYCL>`. | ||
See section :ref:`Installing ASAGI <installing_ASAGI>`. |
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.
Add: If you want to use SeisSol with ASAGI, you need to install
easi with ASAGI support.
Maybe a general remark: I'd add a remark: "This documentation is tested for [Ubuntu 22.04 LTS]." Then we should be sure that the documentation actually works in a fresh installed Ubuntu 22.04 (maybe test with docker). |
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.
thank you for these changes.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
.. _gemmforge_installation: | ||
NetCDF is needed for convergence tests, as these use periodic boundary conditions—and such are not supported by the PUML mesh format. |
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.
typo conditions—and
|
||
.. code-block:: bash | ||
|
||
pip3 install --user git+https://github.com/SeisSol/gemmforge.git | ||
pip3 install --user git+https://github.com/SeisSol/chainforge.git | ||
wget https://syncandshare.lrz.de/dl/fiJNAokgbe2vNU66Ru17DAjT/netcdf-4.6.1.tar.gz |
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.
why a sync and share link?
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.
That was there from before. :) And no one changed it.
cd .. | ||
|
||
(Make sure $HOME/include contains metis.h and $HOME/lib contains | ||
libmetis.a. Otherwise, compile error: cannot find parmetis.) | ||
|
||
Other Software (optional) |
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.
Softwares
|
||
ParMETIS may be installed as follows: | ||
|
||
.. code-block:: bash | ||
|
||
wget https://ftp.mcs.anl.gov/pub/pdetools/spack-pkgs/parmetis-4.0.3.tar.gz | ||
tar -xvf parmetis-4.0.3.tar.gz | ||
cd parmetis-4.0.3 | ||
#edit ./metis/include/metis.h IDXTYPEWIDTH to be 64 (default is 32). |
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 would not rely on the user to read this instruction, and do as here:
https://github.com/SeisSol/SeisSol/blob/master/.ci/docker-gpu/Dockerfile.base#L73
|
||
.. code-block:: bash | ||
Mesh Partitioning (optional, recommended) |
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.
Mesh partitioning library?
Code Generators for CPUs (optional, recommended) | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
For CPU code generators, we support the following: |
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 support the following CPU code generators
- libxsmm (libxsmm\_gemm\_generator) for small matrix multiplications | ||
- PSpaMM (pspamm.py) for small sparse matrix multiplications (required only on Knights Landing or Skylake) | ||
|
||
Note that using Eigen does not result in any additional dependencies, since it is needed in SeisSol anyways. |
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 would remove this sentence, and use Sebastian sentence.
|
||
Once you have SYCL and GemmForge (maybe also ChainForge) ready, you are set for compiling SeisSol with GPUs. | ||
|
||
Code Generators for CPUs (optional, recommended) |
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.
remove for CPUs
(you talk about GPU code generators in this section).
~~~~~~~~~~~~~~~~~~ | ||
"""""""""""""""""" | ||
|
||
(to save data, we only use a shallow clone) |
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 would remove this sentence.
|
||
(to save data, we only use a shallow clone) |
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.
remove
A bit overdue, but we should be able to update our build instructions a bit. So this PR shall do so.
It's still WIP, especially the
Compiling SeisSol
section. Have we documented the CMake parameters anywhere so far? Or will someone need to do that still?