Add a lightweight MPIABI sub-package that controls the MPI ABI #529

vchuravy · 2021-12-17T15:45:07Z

The goal is to have a central source of truth to query the MPI ABI used by Julia and by binary dependencies.

With this we can change the ABI, and after a restart the user will download the appropriate binaries/complain that they are incompatible.

With this I can change JuliaPackaging/Yggdrasil#4073 to contain

augmented_platform_block = """
    using Base.BinaryPlatforms
    using MPIABI
    function augmented_platform(platform)
        abi = MPI.abi
        BinaryPlatforms.add_tag!(platform.tags, "mpi", string(abi))
        return platform
    end
"""

and have the LAMMPS_jll use the right ABI.

Open questions are:

ABI vs System and Library? E.g. Can we assume that binaries built against OpenMPI_jll are compatible with a system OpenMPI? I think so, so switching on the ABI and not the library used "should" work.
Do we need to have a separate package? I think so, we want the choice to be as lightweight as possible, and MPI.jl might use MPIABI to what it includes itself.

cc: @eschnett @staticfloat @simonbyrne

simonbyrne · 2021-12-17T17:25:36Z

Can we assume that binaries built against OpenMPI_jll are compatible with a system OpenMPI?

I seem to recall that the .so version number is baked into the binaries, so maybe not.

simonbyrne · 2021-12-17T19:36:52Z

lib/MPIABI/src/MPIABI.jl

+using Preferences
+
+function known_abis()
+    return (:MicrosoftMPI, :MPICH, :OpenMPI, :MPItrampoline)


should we make these all lowercase?

I think JLLWrapper&Co normalize these to lower-case. I don't have a strong preference.

simonbyrne · 2021-12-17T19:41:34Z

lib/MPIABI/src/MPIABI.jl

+            $(known_abis())
+        """)
+    end
+    @set_preferences!("abi" => abi)


path, library, mpiexec are dependent on which actual MPI installation is being loaded and the intention is for this to be used to choose which MPI installation to be loaded.

The question is: Do we bifurcate along the _jll package used or along the ABI used. Thinking about the library versioning issue I think we need to make this mean with _jll is being used by MPI.jl (which is a shame) I would like it to be the ABI.

Right now if the ABI is set to OpenMPI we will load OpenMPI_jll as part of LAMMPS_jll, so that would not be compatible with binary==system && abi==openmpi IIUC, except if we also set libmpi_path for OpenMPI_jll?

But then we could still run into a library version mismatch?

How do you figure out the ABI without knowing the library etc?

That's why this package doesn't do it. It is a communication channel for ABI aware JLL packages.

I was thinking MPI.jl would figure out the details and then set it here.

Ah: so how would this work? a user would call MPI.set_library(...), which would find the library and set the preferences (in MPI.jl), and also call MPIABI.set_abi(...)?

Yeah, pretty much.

There are some advantages to setting them all in the same package:

it's easier to ensure that they stay in sync (e.g. that the preferences are set in the same location)

it might help avoid an unnecessary round of precompilation

Yeah I making this up here as we go along :)
I don't think we have a good idea of what the best practices should be.

The other extreme is to just use MPI.jl And make all the JLL packages depend on it

I like the idea of a subpackage, but if e.g. Yggdrasil binaries built against MPICH are only valid for MPICH_jll and not the system MPICH, it would make sense to have binary in the subpackage. And once you have that, you might as well put them all in the same place.

giordano · 2021-12-18T01:03:38Z

lib/MPIABI/src/MPIABI.jl

+
+using Preferences
+
+function known_abis()


Out of curiosity, why a function and not a constant?

constants are just 0-arity functions, without participation in the cache invalidation scheme xD

giordano · 2021-12-18T01:04:25Z

lib/MPIABI/src/MPIABI.jl

+const abi = @load_preference("abi", Sys.iswindows() ? :MicrosoftMPI : :MPICH)
+
+function __init__()
+    if Sys.iswindows()


Suggested change

if Sys.iswindows()

@static if Sys.iswindows()

?

giordano · 2021-12-18T01:05:13Z

lib/MPIABI/src/MPIABI.jl

+    @set_preferences!("abi" => abi)
+    @warn "The MPI abi has changed, you will need to restart Julia for the change to take effect" abi
+
+    if VERSION <= v"1.6.5" || VERSION == v"1.7.0"


Maybe @static also here?

giordano · 2021-12-18T01:07:09Z

lib/MPIABI/Project.toml

+version = "0.1.0"
+
+[deps]
+Preferences = "21216c6a-2e73-6563-6e65-726566657250"


Compat for Preferences.jl and Julia

giordano · 2021-12-18T01:08:01Z

lib/MPIABI/Project.toml

@@ -0,0 +1,7 @@
+name = "MPIABI"
+uuid = "3da0fdf6-3ccc-4f1b-acd9-58baa6c99267"
+authors = []


Poor package, no one wants to claim authorship 😔

giordano · 2021-12-18T01:12:04Z

lib/MPIABI/src/MPIABI.jl

+        """
+
+        uuid = Preferences.get_uuid(@__MODULE__)
+        project_toml, pkg_name = Preferences.find_first_project_with_uuid(uuid)


Wouldn't Pkg.add be much easier? But you probably don't want to require Pkg just for a workaround?

I am tempted to fix this in Preferences.jl, which is where I stole this code from.

If version 1.6.5 was already released I would just make that the compat xD

1.6.5 should be very close: https://discourse.julialang.org/t/julia-1-6-5-testing-period/73068

simonbyrne · 2022-01-03T04:44:29Z

I've been thinking about this a bit more, and prompted by @staticfloat's comment here: JuliaInterop/CxxWrap.jl#309 (comment), could we somehow encapsulate all this (and JuliaPackaging/Yggdrasil#4073) by creating a new MPI_jll package?

What I was vaguely thinking is that this would act like an ordinary jll package, but with some extra constants (which could be set by Preferences):

binary: specify which jll package to use, or "system" if using a non-jll binary.
abi: as defined in this PR
libmpi, mpiexec, etc: these would import the appropriate jll package bindings, or if binary == "system", would be set by libmpi_path, etc.

simonbyrne · 2022-01-03T05:14:17Z

Ideally this would also handle the platform PackageSpec stuff (i.e. adding MPI_jll as a PackageSpec would automatically expand the platforms)

vchuravy · 2022-03-06T14:45:27Z

Closed in favor of #541

vchuravy force-pushed the vc/MPIABI branch from 37f18e6 to 50db836 Compare December 17, 2021 16:25

Add a lightweight MPIABI sub-package that controls the MPI ABI

50db836

simonbyrne reviewed Dec 17, 2021

View reviewed changes

giordano reviewed Dec 18, 2021

View reviewed changes

giordano mentioned this pull request Dec 18, 2021

Run CI only when relevant directories are modified #530

Merged

vchuravy mentioned this pull request Dec 21, 2021

Support augment_platform in JLLWrappers JuliaPackaging/JLLWrappers.jl#35

Merged

simonbyrne added this to the 1.0 milestone Jan 25, 2022

simonbyrne mentioned this pull request Jan 25, 2022

Switch to Preferences.jl, remove Pkg.build step #535

Closed

simonbyrne mentioned this pull request Feb 15, 2022

Add MPIPreferences subpackage to select binary #541

Merged

vchuravy closed this Mar 6, 2022

giordano deleted the vc/MPIABI branch September 16, 2022 19:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a lightweight MPIABI sub-package that controls the MPI ABI #529

Add a lightweight MPIABI sub-package that controls the MPI ABI #529

vchuravy commented Dec 17, 2021 •

edited

Loading

simonbyrne commented Dec 17, 2021

simonbyrne Dec 17, 2021

vchuravy Dec 18, 2021

simonbyrne Dec 17, 2021

vchuravy Dec 18, 2021

simonbyrne Dec 18, 2021

vchuravy Dec 18, 2021

simonbyrne Dec 18, 2021

vchuravy Dec 18, 2021

simonbyrne Dec 19, 2021

vchuravy Dec 19, 2021

simonbyrne Dec 22, 2021 •

edited

Loading

giordano Dec 18, 2021

vchuravy Dec 18, 2021

giordano Dec 18, 2021

giordano Dec 18, 2021

giordano Dec 18, 2021

giordano Dec 18, 2021

giordano Dec 18, 2021 •

edited

Loading

vchuravy Dec 18, 2021

vchuravy Dec 18, 2021

giordano Dec 18, 2021

simonbyrne commented Jan 3, 2022

simonbyrne commented Jan 3, 2022

vchuravy commented Mar 6, 2022

Add a lightweight MPIABI sub-package that controls the MPI ABI #529

Add a lightweight MPIABI sub-package that controls the MPI ABI #529

Conversation

vchuravy commented Dec 17, 2021 • edited Loading

simonbyrne commented Dec 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbyrne Dec 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giordano Dec 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbyrne commented Jan 3, 2022

simonbyrne commented Jan 3, 2022

vchuravy commented Mar 6, 2022

vchuravy commented Dec 17, 2021 •

edited

Loading

simonbyrne Dec 22, 2021 •

edited

Loading

giordano Dec 18, 2021 •

edited

Loading