[WIP] HPC improvements #79771
[WIP] HPC improvements #79771
Conversation
|
Very happy to see that lammps finally has a standard build procedure with cmake! |
There was a problem hiding this comment.
Hi @ikervagyok. Thank you for the contribution. It's nice to see that there is more and more HPC activity with Nix. As a general comment: this is a big PR that addresses several different issues.
It may be better to split it it up in several PRs (e.g. lammps, openmpi, slurm). It would also simplify the review if you could briefly comment how you have tested the additional features.
In my experience it is one thing to get things to compile but a completely separate thing to make them work. There is usually some fine tuning involved. I test drive new packages in my own overlay before I upstream them (feel free to copy from the overlay what you need).
| @@ -355,6 +355,7 @@ in | |||
| ExecStart = "${wrappedSlurm}/bin/slurmd"; | |||
| PIDFile = "/run/slurmd.pid"; | |||
| ExecReload = "${pkgs.coreutils}/bin/kill -HUP $MAINPID"; | |||
| LimitMEMLOCK = "infinity"; | |||
There was a problem hiding this comment.
This fixed the issue of slurm not being able to use mpirun, while it worked in a shell. I'll try again to see if i've messed up my testing.
| configureFlags = with stdenv; [ | ||
| "--with-cma" | ||
| "--with-hwloc=${hwloc.dev}" | ||
| "--with-ofi=${libfabric}" |
There was a problem hiding this comment.
What benefit does libfabric bring (I am not familiar with libfabric)?
There was a problem hiding this comment.
I think OmniPath needs libfabric - can't try though, since I have no OmniPath machine in a test setup
There was a problem hiding this comment.
I would prefer to remove it then.
There was a problem hiding this comment.
as a dependency, or the whole package?
There was a problem hiding this comment.
I would leave the whole package out for now since the libfabric is not even built with OmniPath in the present version. We can add that later, when it becomes relevant.
| "--with-cma" | ||
| "--with-hwloc=${hwloc.dev}" | ||
| "--with-ofi=${libfabric}" | ||
| "--with-pmi=${pmix}" |
There was a problem hiding this comment.
I have tried to build with pmix in the past but could not get it work properly with slurm. Did you try if you can run openmpi binaries with srun instead of mpirun?
In general I would say pmix is not strictly necessary as openmpi already works nicely with slurm and detects the resource allocations properly. From how i understand pmix it is designed for large mpi jobs (>100 nodes).
There was a problem hiding this comment.
you're right, i've thought it worked, but i probably mistakenly tested mpirun multiple times?! will look into this.
| "--with-pmi=${pmix}" | ||
| "--with-pmix=${pmix} --enable-install-libpmix" | ||
| "--with-verbs=${rdma-core}" | ||
| "--with-ucx=${ucx}" |
There was a problem hiding this comment.
We (@mekeetsa) are testing UCX with infiniband at the moment (see also https://github.com/markuskowa/NixOS-QChem/blob/ucx/ucx/default.nix). Did you test it with an Infiniband configuration?
There was a problem hiding this comment.
yes, i've tested it and it seems to work (setting up the env with OMPI_MCA_btl=^openib,uct,tcp UCX_TLS=ib,sm,self)
| @@ -29,6 +30,12 @@ in stdenv.mkDerivation rec { | |||
| sha256 = "0ms0zvyxyy3pnx9qwib6zaljyp2b3ixny64xvq3czv3jpr8zf2wh"; | |||
| }; | |||
|
|
|||
| # TODO: Remove these, when a new release gets out | |||
| patches = [ | |||
| (fetchpatch { name = "ucx-1.8-compat-1"; url = "https://github.com/open-mpi/ompi/commit/526775dfd7ad75c308532784de4fb3ffed25458f.patch"; sha256 = "1zfr7ym4303dsnnjqfc1qbnfs0izd8bazxfl57p3dhqsiznd734r"; }) | |||
There was a problem hiding this comment.
I am still confused why there are patches for UCX 1.8 when are building against UCX 1.7.
There was a problem hiding this comment.
Take a look at the two commit messages:
open-mpi/ompi@526775d
open-mpi/ompi@a3026c0
The first one adds support and the second replaces the version bound version > 1.7 with version >= 1.7. They are named that way, to make it clear, that they can be removed together.
There was a problem hiding this comment.
The way I understand the patches the first one fixes a problem with ucx/master (correct me if I am wrong). Since we are not building against ucx 1.8 (or current master) that patch is not needed right now. We should remove it for now.
|
Can we close this PR? Most of the improvements have now been addressed in other PRs. |
Motivation for this change
Want to convince my colleagues, that
NixOS/NixOpsis OK for a test cluster setup which consists of a few AArch64 nodes.LAMMPSis the package I chose to give to my test users, that's why I've converted it toCMakeand added a few extraLAMMPS packagesThings done
Updated stuff and added
libfabric,pmixanducx. Tested with 2 nodes, will go up to 4 tomorrow and maybe some x86 nodes in the next few days.@markuskowa since you seem to be at the center of NixOS-HPC what do you think about these changes? What should be made optional/reworked? I know It's not a polished PR, but I think can be made into something usable.
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)