Subject: Re: [PATCH 3/4] gnu: Add openmpi-5.

From: Ludovic Courtès <[email-masked]>
Date: Fri, 12 Jul 2024 10:40:53 +0200

Hello!

Romain GARBAGE <[email-masked]> skribis:

> * gnu/packages/mpi.scm (openmpi-5): New variable.
>
> Change-Id: I048692b2e928077c9cfa9fb8076ceb356251e1db

[...]

> +(define-public openmpi-5
> +  (package/inherit openmpi

I think it’s a case where (package (inherit openmpi) …) is more
appropriate: ‘package/inherit’ would cause the ‘replacement’ of
‘openmpi’ to be inherited, which is probably not a good idea since it’s
a different version.

> +     (substitute-keyword-arguments (package-arguments openmpi)
> +       ((#:configure-flags flags)
> +        #~(append (list "--enable-prte-prefix-by-default" ;replaces --enable-mpirun-prefix-by-default
> +                        ;; Enable support for the 'Process Management
> +                        ;; Interface for Exascale' (PMIx) used e.g. by
> +                        ;; Slurm for the management communication and
> +                        ;; coordination of MPI processes.
> +                        (string-append "--with-pmix=" #$(this-package-input "openpmix"))
> +                        (string-append "--with-prrte=" #$(this-package-input "prrte"))
> +
> +                        ;; Since 5.x, Infiniband support is provided by ucx.
> +                        (string-append "--with-ucx=" #$(this-package-input "ucx")))
> +                  (filter (lambda (e)
> +                            (and (not (string-contains e "pmi")) ;; Since 5.x, OpenMPI doesn't support PMI/PMI-2
> +                                 ;; Deprecated since 5.x.
> +                                 (not (string-contains e "enable-mpirun-prefix-by-default"))
> +                                 ;; Infiniband support is deprecated and superseded by ucx
> +                                 ;; See https://docs.open-mpi.org/en/main/release-notes/networks.html#miscellaneous-network-notes
> +                                 (not (string-contains e "openib"))))
> +                          #$flags)))

How about listing all the configure flags explicitly rather than
relative to those of 4.x?  I feel like this would be clearer and that
there’s little to be gained by factorizing anyway.

Thanks,
Ludo’.