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

Allow using custom libraries without MPI support #9

Merged
merged 12 commits into from
Oct 11, 2020

Conversation

ranocha
Copy link
Member

@ranocha ranocha commented Oct 9, 2020

This sets up some structure to allow using custom builds of p4est. I wanted to test this using a system provided version of p4est from the Debian repo. However, that one uses MPI and we will have to add all of the relevant MPI includes etc. Hence, I think we should join forces in adding MPI support (#6) and custom p4est builds.

Closes #5

@ranocha ranocha requested a review from sloede October 9, 2020 14:14
@ranocha ranocha marked this pull request as draft October 9, 2020 14:14
@ranocha
Copy link
Member Author

ranocha commented Oct 9, 2020

~/.julia/dev/P4est$ JULIA_P4EST_INCLUDE="/usr/include" JULIA_P4EST_LIBRARY="/usr/lib/x86_64-linux-gnu/libp4est.so" julia --project=@.
julia> import Pkg; Pkg.build("P4est")
   Building P4est  `~/.julia/dev/P4est/deps/build.log`
┌ Error: Error building `P4est`: 
│ /usr/include/sc.h:84:10: fatal error: 'mpi.h' file not found
│ ERROR: LoadError: Errors encountered parsing headers, unable to proceed with conversion
│ Stacktrace:
│  [1] error(::String) at ./error.jl:33
│  [2] (::CBindingGen.var"#18#21"{var"#3#4",Array{String,1}})(::String) at ~/.julia/packages/CBindingGen/44XAK/src/CBindingGen.jl:192
│  [3] mktempdir(::CBindingGen.var"#18#21"{var"#3#4",Array{String,1}}, ::String; prefix::String) at ./file.jl:682
│  [4] mktempdir(::Function, ::String) at ./file.jl:680 (repeats 2 times)
│  [5] convert_headers(::Function, ::Array{String,1}; args::Array{String,1}) at ~/.julia/packages/CBindingGen/44XAK/src/CBindingGen.jl:168
│  [6] top-level scope at ~/.julia/dev/P4est/deps/build.jl:120
│  [7] include(::String) at ./client.jl:457
│  [8] top-level scope at none:5in expression starting at ~/.julia/dev/P4est/deps/build.jl:120
│ Use custom p4est library /usr/lib/x86_64-linux-gnu/libp4est.so
│ Use custom p4est include path /usr/include
│ include_directories = ["/usr/include"]
│ include_args = ["-I", "/usr/include"]
└ @ Pkg.Operations /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Pkg/src/Operations.jl:949

@sloede
Copy link
Member

sloede commented Oct 9, 2020

Thanks for getting the ball rolling here! Could you try it out with a custom p4est installation without MPI? This should get you up and running:

P4EST_TMP=~/code/p4est/tmp # modify path to your favorite location
mkdir -p $P4EST_TMP
cd $P4EST_TMP/
wget https://p4est.github.io/release/p4est-2.2.tar.gz
tar xf p4est-2.2.tar.gz
mkdir build
cd build/
$P4EST_TMP/p4est-2.2/configure --prefix=$P4EST_TMP/prefix
make -j 4
make install
ls -l $P4EST_TMP/prefix/lib/libp4est.so
# Test by running `$P4EST_TMP/prefix/bin/p4est_step1` (warning: will create some output files in the CWD)

Copy link
Member

@sloede sloede left a comment

Choose a reason for hiding this comment

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

Thanks a lot for getting the ball rolling on this. If you want me to take over some parts, please let me know. In any case I'd be interested to get your feedback on some issues I commented on.

deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Outdated Show resolved Hide resolved
@ranocha
Copy link
Member Author

ranocha commented Oct 11, 2020

Thanks for getting the ball rolling here! Could you try it out with a custom p4est installation without MPI? This should get you up and running:

P4EST_TMP=~/code/p4est/tmp # modify path to your favorite location
mkdir -p $P4EST_TMP
cd $P4EST_TMP/
wget https://p4est.github.io/release/p4est-2.2.tar.gz
tar xf p4est-2.2.tar.gz
mkdir build
cd build/
$P4EST_TMP/p4est-2.2/configure --prefix=$P4EST_TMP/prefix
make -j 4
make install
ls -l $P4EST_TMP/prefix/lib/libp4est.so
# Test by running `$P4EST_TMP/prefix/bin/p4est_step1` (warning: will create some output files in the CWD)

Thanks for that! Using that setup and some modifications that I've just pushed, I can build and test P4est successfully:

~/.julia/dev/P4est$ JULIA_P4EST_LIBRARY=~/.julia/dev/P4est/p4est_tmp/prefix/lib/libp4est-2.2.so  JULIA_P4EST_INCLUDE=~/.julia/dev/P4est/p4est_tmp/prefix/include/ julia --project=@.
julia> import Pkg; Pkg.build("P4est")
   Building P4est  `~/.julia/dev/P4est/deps/build.log`

julia> Pkg.test("P4est")
...
Test Summary:  | Pass  Total
P4est.jl tests |    3      3
    Testing P4est tests passed 

@ranocha ranocha marked this pull request as ready for review October 11, 2020 08:41
@ranocha ranocha requested a review from sloede October 11, 2020 08:47
@ranocha ranocha changed the title WIP: Allow using custom libraries Allow using custom libraries without MPI support Oct 11, 2020
@codecov-io
Copy link

codecov-io commented Oct 11, 2020

Codecov Report

Merging #9 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master        #9   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee16309...38570ef. Read the comment docs.

deps/build.jl Outdated Show resolved Hide resolved
deps/build.jl Show resolved Hide resolved
@ranocha
Copy link
Member Author

ranocha commented Oct 11, 2020

We now have the additional option JULIA_P4EST_PATH and builds on Travis with both P4est_jll and a custom build of p4est.

Copy link
Member

@sloede sloede left a comment

Choose a reason for hiding this comment

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

With the one remark about p6est fixed, this LGTM. Also, you should make yourself the second author I think.

@ranocha ranocha merged commit 03ed236 into trixi-framework:master Oct 11, 2020
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

Successfully merging this pull request may close these issues.

Allow to substitute p4est library with system-installed library without MPI support
3 participants