Skip to content
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

Issue with P4est_jll and MPIPreferences #87

Closed
JoshuaLampert opened this issue Oct 4, 2023 · 10 comments
Closed

Issue with P4est_jll and MPIPreferences #87

JoshuaLampert opened this issue Oct 4, 2023 · 10 comments

Comments

@JoshuaLampert
Copy link
Member

Let me bring up the following issue again. MWE: In a fresh environment with julia --project=., do

julia> using Pkg
julia> Pkg.add(["MPIPreferences", "P4est_jll"])
[...]
julia> using MPIPreferences
julia> MPIPreferences.use_system_binary()
┌ Info: MPI implementation identified
│   libmpi = "libmpi"
│   version_string = "Open MPI v4.1.0, package: Debian OpenMPI, ident: 4.1.0, repo rev: v4.1.0, Dec 18, 2020\0"
│   impl = "OpenMPI"
│   version = v"4.1.0"
└   abi = "OpenMPI"
┌ Info: MPIPreferences changed
│   binary = "system"
│   libmpi = "libmpi"
│   abi = "OpenMPI"
│   mpiexec = "mpiexec"
│   preloads = Any[]
└   preloads_env_switch = nothing
julia> using P4est_jll
julia> P4est_jll.libp4est
"~/.julia/artifacts/3a2ff5d867b2d3d729a84fb3f736761ae336fcbf/lib/libp4est.so"

but after leaving and restarting the REPL julia --project=.:

julia> using P4est_jll
julia> P4est_jll.libp4est
ERROR: UndefVarError: `libp4est` not defined
Stacktrace:
 [1] getproperty(x::Module, f::Symbol)
   @ Base ./Base.jl:31
 [2] top-level scope
   @ REPL[2]:1

After deleting the LocalPreferences.toml, of course, this works again.
This has not been a real issue as one usually wants to use a custom p4est version (and not the one from P4est_jll) when using a system MPI anyway, but the reason I come up with this again is that the exact same steps as above, but with HDF5_jll and libhdf5 does not give any UndefVarError for me, as I would also expect. If this would work for P4est_jll, we can think about adding a convenience function set_libraries! (or similar) that sets the preferences for P4est.jl as it is done in JuliaIO/HDF5.jl#1115. However, with the error above using P4est does not work as long as we have MPIPreferences set and so the convenience function could not be called. Do we know why this happens with P4est_jll in the first place? Does it also happen with a local MPICH version (which is the default for P4est_jll.jl)? And even better: Can we fix this for P4est_jll?
This would also make https://trixi-framework.github.io/P4est.jl/stable/troubleshooting/ obsolete.

@ranocha
Copy link
Member

ranocha commented Oct 5, 2023

Good question. I'm not really familiar with the build setup of HDF5_jll. How do they handle MPI?

@sloede
Copy link
Member

sloede commented Oct 5, 2023

I think this happens because we do not build P4est_jll for OpenMPI, see
https://github.com/JuliaPackaging/Yggdrasil/blob/c26186fc6e01678668521f0be0594be6f8b81fd3/P/P4est/build_tarballs.jl#L80-L82

That means if we use P4est_jll as-is, loading a different MPI library than MPICH will cause P4est_jll to not find a suitable artifact and thus P4est_jll.libp4est remains unset. The "correct" fix would here be to enable building p4est with BinaryBuilder.jl also for OpenMPI, i.e., modifying the build_tarballs.jl and possibly some sources of p4est to play nice with OpenMPI.

@JoshuaLampert if you're willing to take a stab at this, I can guide you through using BB.jl and the build_tarballs.jl to try to fix this.

@JoshuaLampert
Copy link
Member Author

Thanks, @sloede! I was thinking something along those lines since HDF5_jll cross-compiles for all MPI implementations and P4est_jll does not.
I would definitely like to fix this properly, but I am not sure how much work this would be (especially for someone like me who has only limited experience with BB.jl). Looking at JuliaPackaging/Yggdrasil#6551 this looks quite heavy... But if you think it is doable with your guidance @sloede, I would be happy to give it a try!

@sloede
Copy link
Member

sloede commented Oct 5, 2023

The first things are super easy:

  • fork Yggdrasil
  • clone the fork on roci and create a branch
  • modify the p4est build_tarballs.jl and comment the line that disables OpenMPI
  • create a draft PR to Yggdrasil

Now you wait and see until Yggdrasil has built all configurations and you check how many of them fail. Then, pick the "easiest" sounding config that fails (for example, Windows is not easy, whereas x86 Linux might be) and try to build it locally:

  • go to your Yggdrasil clone into the directory where the build_tarballs.jl is located
  • install BB: julia --project=. -e 'using Pkg; Pkg.add("BinaryBuilder")'
  • run a config that should work to verify that BB is working in general by executing
    julia --project=. build_tarballs.jl --deploy=local --verbose --debug=error x86_64-linux-gnu-mpi+mpich
  • if that works (it should), pick a platform that does not work and modify the last argument to the command above
  • the build will fail at some point and BB drops you into a local sandboxed environment. it's like a Unix shell and you can now muck around and try to rebuild etc. if you want to start a fresh session, you can change it to --debug=begin to be dropped at the beginning of the build session

After that, I think it's better to ask specific questions :-)

@JoshuaLampert
Copy link
Member Author

Thanks a lot for the detailed description @sloede . I'll try it tomorrow.

@JoshuaLampert
Copy link
Member Author

JoshuaLampert commented Oct 6, 2023

Ok, a new version of P4est_jll is out (JuliaPackaging/Yggdrasil#7500), which also builds OpenMPI (that was easier than expected :-)). So this should be fixed (at least for the most common systems).
What do you think of a convenience function set_p4est_preference! (and set_sc_preference!) that sets the preference easier without the need to copy/paste the UUID of P4est? If that works, I also suggest to do something similar for T8code.jl.

@ranocha
Copy link
Member

ranocha commented Oct 6, 2023

If that works, it's a nice addition. It's just important to restart Julia since it's a compile-time preference, isn't it?

@JoshuaLampert
Copy link
Member Author

Yes you would still need to restart Julia.

@sloede
Copy link
Member

sloede commented Oct 6, 2023

Good idea! If HDF5 started with set_library!, what about following along with set_library_p4est! etc? Though I don't have strong preferences here (no pun intended). One should show an info message though that Julia needs to be restarted.

@JoshuaLampert
Copy link
Member Author

See my PR #88. I used a similar pattern as in HDF5.jl.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants