Support MPItrampoline as MPI provider

[eschnett]

This changes MPI.jl to accept MPItrampoline as MPI provider.

Since MPItrampoline is initialized later than other MPI implementations (at load time, not at build time), MPI.jl's initialization of various constants also needs to be delayed when MPItrampoline is used. I currently convert const to global objects for this. It would be possible to use const Ref instead as well.

This PR also corrects a few places where MPI.jl deviates from the MPI standard, notably when handling MPI_Status.

[eschnett]

Closes #502

[simonbyrne]

This is awesome, let me know if there is anything I can do to help.

[eschnett]

@simonbyrne At the moment I'm fighting with the unit tests. Locally and on HPC systems the tests work fine.

I'm looking for general feedback, in particular for the larger changes I'm making outside the code that is MPItrampoline-only.

[vchuravy]

Would it make sense to split those changes into two?

Right now it looks like the two issues are that MPItrampoline_jll will only work on Julia 1.6 and

ERROR: LoadError: MethodError: no method matching unsafe_convert(::Type{Ptr{Nothing}}, ::MPI.SentinelPtr)
Closest candidates are:
  unsafe_convert(::Union{Type{Ptr{Nothing}}, Type{Ptr{Base.Libc.FILE}}}, !Matched::Base.Libc.FILE) at libc.jl:94
  unsafe_convert(::Type{Ptr{Tv}}, !Matched::SuiteSparse.CHOLMOD.Sparse{Tv}) where Tv at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/SuiteSparse/src/cholmod.jl:296
  unsafe_convert(::Type{Ptr{T}}, !Matched::LinearAlgebra.Transpose{var"#s814", var"#s813"} where {var"#s814", var"#s813"<:(AbstractVecOrMat{T} where T)}) where T at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/adjtrans.jl:222
  ...
Stacktrace:
 [1] create_keyval(#unused#::Type{MPI.Datatype})
   @ MPI ~/work/MPI.jl/MPI.jl/src/datatypes.jl:46
 [2] (::MPI.var"#17#18")()
   @ MPI ~/work/MPI.jl/MPI.jl/src/datatypes.jl:93
 [3] run_init_hooks()
   @ MPI ~/work/MPI.jl/MPI.jl/src/environment.jl:49
 [4] Init(; threadlevel::Symbol, finalize_atexit::Bool, errors_return::Bool)
   @ MPI ~/work/MPI.jl/MPI.jl/src/environment.jl:111
 [5] Init()
   @ MPI ~/work/MPI.jl/MPI.jl/src/environment.jl:82
 [6] top-level scope
   @ ~/work/MPI.jl/MPI.jl/docs/examples/01-hello.jl:2

[vchuravy]

diff --git a/deps/consts_openmpi.jl b/deps/consts_openmpi.jl
index 0face3e..d84c202 100644
--- a/deps/consts_openmpi.jl
+++ b/deps/consts_openmpi.jl
@@ -132,3 +132,20 @@ const MPI_IN_PLACE = reinterpret(SentinelPtr, 1)
 const MPI_STATUS_IGNORE = reinterpret(SentinelPtr, 0)
 const MPI_STATUSES_IGNORE = reinterpret(SentinelPtr, 0)
 
+function _type_null_copy_fn(oldtype::MPI_Datatype, type_keyval::Cint,
+                            extra_state::Ptr{Cvoid},
+                            attribute_val_in::Ptr{Cvoid}, attribute_val_out::Ptr{Cvoid},
+                            flag::Ptr{Cint})
+    unsafe_store!(flag, Cint(0))
+    return MPI_SUCCESS
+end
+function _type_null_delete_fn(oldtype::MPI_Datatype, type_keyval::Cint,
+                           attribute_val_in::Ptr{Cvoid}, extra_state::Ptr{Cvoid})
+    return MPI_SUCCESS
+end
+
+const MPI_TYPE_NULL_COPY_FN = @cfunction(_type_null_copy_fn,Cint,(MPI_Datatype, Cint, Ptr{Cvoid},
+                                                                   Ptr{Cvoid}, Ptr{Cvoid}, Ptr{Cint}))
+
+const MPI_TYPE_NULL_DELETE_FN = @cfunction(_type_null_delete_fn,Cint,(MPI_Datatype, Cint, Ptr{Cvoid},
+                                                       Ptr{Cvoid}))
\ No newline at end of file
diff --git a/src/datatypes.jl b/src/datatypes.jl
index acbba29..0ecce2b 100644
--- a/src/datatypes.jl
+++ b/src/datatypes.jl
@@ -29,17 +29,6 @@ function free(dt::Datatype)
 end
 
 # attributes
-# function _type_null_copy_fn(oldtype::MPI_Datatype, type_keyval::Cint,
-#                             extra_state::Ptr{Cvoid},
-#                             attribute_val_in::Ptr{Cvoid}, attribute_val_out::Ptr{Cvoid},
-#                             flag::Ptr{Cint})
-#     unsafe_store!(flag, Cint(0))
-#     return MPI_SUCCESS
-# end
-# function _type_null_delete_fn(oldtype::MPI_Datatype, type_keyval::Cint,
-#                            attribute_val_in::Ptr{Cvoid}, extra_state::Ptr{Cvoid})
-#     return MPI_SUCCESS
-# end
 
 function create_keyval(::Type{Datatype})
     ref = Ref(Cint(0))
@@ -48,10 +37,6 @@ function create_keyval(::Type{Datatype})
                    Ptr{Cvoid},
                    Ptr{Cint},
                    Ptr{Cvoid}),
-                  # @cfunction(_type_null_copy_fn,Cint,(MPI_Datatype, Cint, Ptr{Cvoid},
-                  #                                     Ptr{Cvoid}, Ptr{Cvoid}, Ptr{Cint})),
-                  # @cfunction(_type_null_delete_fn,Cint,(MPI_Datatype, Cint, Ptr{Cvoid},
-                  #                                     Ptr{Cvoid})),
                   MPI_TYPE_NULL_COPY_FN,
                   MPI_TYPE_NULL_DELETE_FN,
                   ref, C_NULL)

made me at least get through OpenMPI until I hit the new errhandler changes.

[eschnett]

Regarding splitting the changes into two: Yes, that would be possible. I'd like to wait at least until things work so that the changes I need to make until then don't break when MPItrampoline is used.

The changes that add support for MPItrampoline are inactive by default. You need to set binary = "MPItrampoline_jll" to activate them. If I can figure out a way to support depending on MPItrampoline_jll on Julia versions other than 1.6 then these changes should be harmless.

[eschnett]

The PR is ready. There is 1 approval. It's a PR, though. Should I expect a second approval? And: When do we increase the version number?

[simonbyrne]

Thanks @eschnett, this is going to be a huge improvement. Did you want to clean up the history, or should I just squash-merge it?

Don't worry about the version for now: we need to bump it anyway, and there are a couple more changes I wanted to make before the next release.

[lcw]

This is awesome! Thanks for all of the hard work @eschnett. Are there any examples of building external packages for Julia via Yggdrasil that run with MPItrampoline? I would love to add a p4est binary in Yggdrasil that does this.

[vchuravy]

This is awesome! Thanks for all of the hard work @eschnett. Are there any examples of building external packages for Julia via Yggdrasil that run with MPItrampoline? I would love to add a p4est binary in Yggdrasil that does this.

Yes! I jumped the gun and built LAMMPS against MPItrampoline a while back https://github.com/JuliaPackaging/Yggdrasil/blob/16bf87457e7b22578244738e6146b55e81acd37a/L/LAMMPS/build_tarballs.jl#L64

[eschnett]

A squash merge is fine.

[sloede]

I can only repeat what others have already written and said: Great work, I am looking forward to having this crucial piece for a Julian approach to HPC usability in MPI.jl!

I would love to add a p4est binary in Yggdrasil that does this.

@lcw We should maybe coordinate on this. Based on the discussion with @eschnett I had last month, we (@lchristm is doing most of the work) are working on building p4est in P4est_jll against MicrosoftMPI on Win* and against MPICH elsewhere (the respective default MPI versions on each platform). I think this still makes sense as long as MPItrampoline is not the default MPI version on *nix-like systems, since otherwise users would always have to switch out the used MPI library and cannot just use P4est_jll out of the box. Or am I missing something here? (we should probably continue this discussion elsewhere, since its not pertinent to this PR - sorry for the spam!)

[vchuravy]

and against MPICH elsewhere

I am wondering if we can use MPITrampoline already since otherwise you will get: JuliaParallel/Elemental.jl#75 (comment) and that's almost a worse experience.. Let me experiment with LAMMPS_jll

[lcw]

@sloede: Let's see what @vchuravy comes up with for LAMMPS_jll. I am hoping we can use MPITrampoline already and maybe distribute an MPIwrapper for MPICH_jll to get things to work out of the box. We might need to modify mpiexecjl to make it smooth for the novice user.

[eschnett]

@sloede In principle we can switch MPI.jl to using MPItrampoline_jll by default right away. Currently, you have to set .julia/prefs/MPI.toml to binary = "MPItrampoline_jll" to do so.

Yggdrasil builds MPItrampoline together with a wrapped MPICH for convenience, and configures MPItrampoline to use that MPICH as fallback if no other MPI implementation is selected. That is, if you use MPItrampoline_jll and do nothing else, you get MPICH (same as before), except that all calls are routed via MPItrampoline.

I didn't suggest doing so right away because I wanted to give people an opportunity to test the MPItrampoline_jll before switching the default. On the other hand, changing defaults is the best way to get quick feedback, and this is "just" about development version of MPI.jl, so maybe we should do that change, just to see what breaks.

[sloede]

I agree - the best test would be to just use it. However, I assume that you have already run a lot of tests yourself, and we have CI here, so more academic tests would probably add not much to it. Thus if we switch to MPItrampoline_jll by default on *nix platforms, I say we should immediately do a release and put a notice in the README and the docs that this might potentially break stuff (and explain how people can restrict their MPI.jl package dependency to a prior version). Then we try to fix it quickly and re-release. If we only keep it in main, I don't think many people will actually test it in real applications.

However, considering the likely decrease in developer availability over the upcoming holiday season (and thus longer reaction times in case something does break), I suggest to wait with merging such a MPItrampoline-is-the-new-default PR until the second week of January.

[eschnett]

I suggest we first make the change on the master branch to give you and a few others a quick chance to check that I didn't overlook an important mode of operation (or a lack of documentation that confuses others), and then release a new version.

I'll create a PR to change the default, to be applied either now or after the winter break.

[sloede]

Sounds good to me 👍