-
-
Notifications
You must be signed in to change notification settings - Fork 122
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
name = "MPIABI" | ||
uuid = "3da0fdf6-3ccc-4f1b-acd9-58baa6c99267" | ||
authors = [] | ||
version = "0.1.0" | ||
|
||
[deps] | ||
Preferences = "21216c6a-2e73-6563-6e65-726566657250" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compat for Preferences.jl and Julia |
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/> |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,79 @@ | ||||||
module MPIABI | ||||||
|
||||||
using Preferences | ||||||
|
||||||
function known_abis() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, why a function and not a constant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. constants are just |
||||||
return (:MicrosoftMPI, :MPICH, :OpenMPI, :MPItrampoline) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we make these all lowercase? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
? |
||||||
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) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In #524, I suggested the following:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The question is: Do we bifurcate along the Right now if the ABI is set to But then we could still run into a library version mismatch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you figure out the ABI without knowing the library etc? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I was thinking MPI.jl would figure out the details and then set it here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah: so how would this work? a user would call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, pretty much. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some advantages to setting them all in the same package:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I making this up here as we go along :) The other extreme is to just use MPI.jl And make all the JLL packages depend on it There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||
@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" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||||||
@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) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1.6.5 should be very close: https://discourse.julialang.org/t/julia-1-6-5-testing-period/73068 |
||||||
|
||||||
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 |
There was a problem hiding this comment.
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 😔