[bug#31080] [PATCH 0/2] Add launchmon and spindle

[Eric Bavier]

On Mon, Apr 09, 2018 at 03:50:34PM +0200, Ludovic Courtès wrote:
> I should say that this is Pierre-Antoine’s first week as an intern here
> at Inria to work on Guix for HPC.  So welcome, Pierre-Antoine!  :-)

Exciting, welcome Pierre-Antoine!  It will be nice to have another
person on Guix for HPC.

> The patches look good to me, but as Pierre-Antoine wrote, we’d like to
> get feedback from someone more familiar with the tools.  Eric maybe?

I have not used these tools much before, but I'll see if I can give
them a test run.  It will be a good excuse for me to finally get Guix
installed on one of our x86 systems. :)

Just a few comments/nitpicks on the patches:

> +
> +(define-public launchmon
> +  (package
> +    (name "launchmon")
> +    (version "1.0.2")
> +    (source (origin
> +              (file-name (git-file-name name version))
                  ^
Leave this field out, since the tarball file-name is already fine.

> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://github.com/LLNL/LaunchMON/releases/download/v";
> +                    version "/" name "-v" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "0fm3nd9mydm9v2bf7bh01dbgrfnpwkapxa3dsvy3x1z0rz61qc0x"))))
> +    (build-system gnu-build-system)
> +    (inputs `(("munge" ,munge)
               ^
Please indent the inputs list like the other packages in this file.

> +              ("boost" ,boost)
> +              ("libelf" ,libelf)
> +              ("openmpi" ,openmpi)

IDK if this would be meaningful for these tools, but some other MPI
packages have used the convention of of naming this input just "mpi".
IIRC Ludovic suggested this in a paper.

> +              ("libgcrypt" ,libgcrypt)
> +              ("libgpg-error" ,libgpg-error)))
> +    (synopsis "Software infrastructure for HPC environments")

Maybe: "Launch tools into HPC environments" so as not to confuse it
with a general piece of "software infrastructure"

> +    (description
> +     "LaunchMON is a software infrastructure that enables HPC
> +(High-Performance Computing) run-time tools to co-locate tool daemons with a
> +parallel job.  Its API allows a tool to identify all the remote processes of 
> a
> +job and to scalably launch daemons into the relevant nodes.")
> +    (home-page "https://github.com/LLNL/LaunchMON";)
> +    (license license:lgpl2.1)))

We should also include a "supported-systems" field here:

   (supported-systems '("i686-linux" "x86_64-linux"))

Since only x86 and powerpc architectures are supported, as noted
above. :(

+
+(define-public spindle
+  (package
+    (name "spindle")
+    (version "0.10")
+    (source (origin
+              (file-name (git-file-name name version))
+              (method git-fetch)

Maybe just note in a comment that we're using a git checkout to avoid
auto-generated github tarballs.

+              (uri (git-reference
+                    (url "https://github.com/hpc/Spindle.git";)
+                    (commit "ff922c1df167c28ab312d02217ed7be65ec0e831")))
                              ^
Could we reference the 'v0.10' tag instead?

+              (sha256
+               (base32
+                "15n3ay0qq81r5v7fif61q1vdjcq44pp2nynkh3fvbzc9fj3c39wd"))))
+    (build-system gnu-build-system)
+    (arguments '(#:configure-flags '("--enable-sec-launchmon"
+                                     "--enable-sec-munge"
+                                     "--enable-sec-none")))
+    (inputs `(("munge" ,munge)
             ^
nit: indentation

+              ("openmpi" ,openmpi)
+              ("launchmon" ,launchmon)
+              ("libgcrypt" ,libgcrypt)))
+    (synopsis "Scalable dynamic library and Python loading in HPC
+environments")

Maybe: "Scalable library loading in HPC environments"

+    (description
+     "Spindle is a tool for improving the performance of dynamic library and
+Python loading in HPC environments.")
+    (home-page "https://github.com/hpc/Spindle";)
+    (license license:lgpl2.1)))


Otherwise LGTM.  I'll see if I can give them a try soon.

-- 
Eric Bavier, Scientific Libraries, Cray Inc.