Skip to content

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

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions lib/MPIABI/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
name = "MPIABI"
uuid = "3da0fdf6-3ccc-4f1b-acd9-58baa6c99267"
authors = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poor package, no one wants to claim authorship 😔

version = "0.1.0"

[deps]
Preferences = "21216c6a-2e73-6563-6e65-726566657250"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compat for Preferences.jl and Julia

25 changes: 25 additions & 0 deletions lib/MPIABI/UNLICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
This is free and unencumbered software released into the public
domain.

Anyone is free to copy, modify, publish, use, compile, sell, or
distribute this software, either in source code form or as a compiled
binary, for any purpose, commercial or non-commercial, and by any
means.

In jurisdictions that recognize copyright laws, the author or authors
of this software dedicate any and all copyright interest in the
software to the public domain. We make this dedication for the benefit
of the public at large and to the detriment of our heirs and
successors. We intend this dedication to be an overt act of
relinquishment in perpetuity of all present and future rights to this
software under copyright law.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR
OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
OTHER DEALINGS IN THE SOFTWARE.

For more information, please refer to <http://unlicense.org/>
79 changes: 79 additions & 0 deletions lib/MPIABI/src/MPIABI.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
module MPIABI

using Preferences

function known_abis()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return (:MicrosoftMPI, :MPICH, :OpenMPI, :MPItrampoline)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make these all lowercase?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

end

# const abi = begin
# _abi = @load_preference("abi", nothing)
# if isnothing(_abi)
# _abi = find_abi()
# @set_preference!("abi" => _abi)
# end
# _abi
# end

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

function __init__()
if Sys.iswindows()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if Sys.iswindows()
@static if Sys.iswindows()

?

if abi != :MicrosoftMPI
@error """
The MPI ABI is not compatible with the current platform.
Please set the MPI ABI to :MicrosoftMPI.
"""
end
end
end

function set_abi(abi)
if abi ∉ known_abis()
error("""
The MPI ABI $abi is not supported.
Please set the MPI ABI to one of the following:
$(known_abis())
""")
end
@set_preferences!("abi" => abi)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #524, I suggested the following:

#  binary   = "" (default) | "system" | "MPICH_jll" | "OpenMPI_jll" | "MicrosoftMPI_jll"
#  path     = "" (default) | top-level directory location
#  library  = "" (default) | library name/path | list of library names/paths
#  abi      = "" (default) | "MPICH" | "OpenMPI" | "MicrosoftMPI" | "unknown"
#  mpiexec  = "" (default) | executable name/path

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(...)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, pretty much.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@simonbyrne simonbyrne Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @static also here?

@warn """
Due to a bug in Julia (until 1.6.5 and 1.7.1), setting preferences in transitive dependencies
is broken (https://github.com/JuliaPackaging/Preferences.jl/issues/24). As a workaround, we
will now add MPIABI as a direct dependency in your current project.
"""

uuid = Preferences.get_uuid(@__MODULE__)
project_toml, pkg_name = Preferences.find_first_project_with_uuid(uuid)
Copy link
Member

@giordano giordano Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


if project_toml === nothing && pkg_name === nothing
# If we couldn't find one, we're going to use `active_project()`
project_toml = Base.active_project()

# And we're going to need to add this UUID as an "extras" dependency:
# We're going to assume you want to name this this dependency in the
# same way as it's been loaded:
pkg_uuid_matches = filter(d -> d.uuid == uuid, keys(Base.loaded_modules))
if isempty(pkg_uuid_matches)
error("Cannot set preferences of an unknown package that is not loaded!")
end
pkg_name = first(pkg_uuid_matches).name

# Read in the project, add the deps, write it back out!
project = Base.parsed_toml(project_toml)
if !haskey(project, "deps")
project["deps"] = Dict{String,Any}()
end
project["deps"][pkg_name] = string(uuid)
open(project_toml, "w") do io
TOML.print(io, project; sorted=true)
end
end
end
return abi
end

end # module