Add access to libsc

[lcw]

Fix calling sc_ functions on Windows (closes #32) and allow access to global C variables in libsc.

[codecov]

Codecov Report

Merging #73 (f38ca9b) into main (bdb6b84) will increase coverage by 0.40%.
The diff coverage is 1.69%.

@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   15.02%   15.42%   +0.40%     
==========================================
  Files           3        3              
  Lines        1511     1511              
==========================================
+ Hits          227      233       +6     
+ Misses       1284     1278       -6     

Flag	Coverage Δ
unittests	15.42% <1.69%> (+0.40%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/P4est.jl	92.31% <ø> (ø)
src/LibP4est.jl	13.91% <1.69%> (+0.41%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ranocha]

Thanks a lot for the contribution, @lcw! I just have a few questions.

• Which change fixed calling libsc functions on Windows?
• As far as I understand, we need to specify also a path to libsc if we want to use a custom build of MPI (and p4est), correct? Thus, this would be a breaking change since it was enough to specify the path to the p4est library before, correct?

I'm writing this on my phone, so I can't test it myself right now.

[lcw]

I don't know windows well, but it seems you need to use @ccall with a path to libsc (not p4est) to call the sc_* functions.

This is not needed on Linux, I am guessing that is because the libp4est shared library brings in libsc. See the following ldd output.

$ ldd libp4est.so
        linux-vdso.so.1 (0x00007ffe03dc5000)
        libsc.so.0 => /home/lucas/research/code/julia/TryP4est/p4est-2.8/local/lib/libsc.so.0 (0x00007f1dfa82d000)
        libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f1dfa7f9000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f1dfa710000)
        libmpi.so.40 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/openmpi-4.1.4-bcr3kmjansftmrl6n2ocydorlpxsakfg/lib/libmpi.so.40 (0x00007f1dfa3e1000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f1dfa1dc000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f1dfa92f000)
        libopen-rte.so.40 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/openmpi-4.1.4-bcr3kmjansftmrl6n2ocydorlpxsakfg/lib/libopen-rte.so.40 (0x00007f1dfa0b4000)
        libopen-pal.so.40 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/openmpi-4.1.4-bcr3kmjansftmrl6n2ocydorlpxsakfg/lib/libopen-pal.so.40 (0x00007f1df9f94000)
        libhwloc.so.15 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/hwloc-2.8.0-emjzr2bhjyjcelbewg7t5ztph25mj2sy/lib/libhwloc.so.15 (0x00007f1df9f32000)
        libevent_core-2.1.so.7 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/libevent-2.1.12-xhiifcklrl2bdfqg35qbgtg46dupqj55/lib/libevent_core-2.1.so.7 (0x00007f1df9ef8000)
        libpmix.so.2 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/pmix-4.1.2-4gp6wpcddf3i5rqwtj7lel3etb2uy7nt/lib/libpmix.so.2 (0x00007f1df9cfe000)
        libevent_pthreads-2.1.so.7 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/libevent-2.1.12-xhiifcklrl2bdfqg35qbgtg46dupqj55/lib/libevent_pthreads-2.1.so.7 (0x00007f1df9cf9000)
        libpciaccess.so.0 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/libpciaccess-0.16-4jz2u4qamleprpgiyz7wjxxanerfzu6t/lib/libpciaccess.so.0 (0x00007f1df9ceb000)
        libcudart.so.11.0 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/cuda-11.8.0-mrm5vsjxqwzax5kqx5kzo6ti6rcj7cze/lib64/libcudart.so.11.0 (0x00007f1df9a00000)
        libxml2.so.2 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/libxml2-2.10.1-vtiyuxhc7vklerekxpntyin5tzko6jdh/lib/libxml2.so.2 (0x00007f1df9894000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f1df9ce6000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f1df9ce1000)
        librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f1df9cda000)
        liblzma.so.5 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/xz-5.2.7-5l2gjavdnb2y4c6ctxu2hpsgv7hypmr7/lib/liblzma.so.5 (0x00007f1df9cb1000)
        libiconv.so.2 => /tmp/scratch/lucas/spack/opt/spack/linux-ubuntu22.10-x86_64_v4/gcc-10.4.0/libiconv-1.16-kl7eftispcsnhbvzj7lcrroiuwhr3m4n/lib/libiconv.so.2 (0x00007f1df9795000)

On all platforms you need to load libsc to get global variables defined in that library. So, it is useful in general to have libsc available. For example, this can be used to see if libsc has been initialized.

sc_package_id() = unsafe_load(cglobal((:sc_package_id, P4est.LibP4est.libsc), Cint))
initialized() = sc_package_id() >= 0

Yes, this is a breaking change as you do need to specify a path to libsc for a custom build of p4est (and libsc).

[ranocha]

Great, thanks a lot, @lcw!

@sloede @JoshuaLampert: If you don't object, I will merge this and create a breaking release.

[sloede]

Right now, we handle setting custom p4est/sc library versions by using a manual mechanism to change what libp4est (now also libsc) means within P4est.jl. I recently stumbled across this note in the BB docs, that says you can already do that for JLL packages out of the box, i.e., override the path of a given artifact:
https://docs.binarybuilder.org/stable/jll/#Overriding-specific-products

I didn't see it as a critical change, but now that we're creating a breaking change that addresses the way in which we set custom library paths, I thought I'd bring it up.

IMHO it's fine to continue with this PR here. Should it be clear that overriding the JLL packages would be clearly superior to our current way of doing it, I would suggest to apply that change as well before creating the breaking release.

[sloede]

Thanks for the nice improvement, @lcw!

[ranocha]

Thanks, @sloede! I didn't know about this feature of JLL packages. We should meet soon to discuss what to include in the next breaking release of P4est.jl. Anyway, I like the improvements from this PR.

[JoshuaLampert]

Indeed, this looks like a nice improvement! As far as I understand, after merging this, on Linux you could set libsc to the same path as libp4est (or to a custom build of libsc if you like), right? So maybe it would be convenient to set the default value of _PREFERENCE_LIBSC to _PREFERENCE_LIBP4EST, which means that on Linux you would not need to set both, libp4est and libsc, but setting libp4est is enough. Of course, then we should also document this properly. Any thoughts on this?

[sloede]

Indeed, this looks like a nice improvement! As far as I understand, after merging this, on Linux you could set libsc to the same path as libp4est (or to a custom build of libsc if you like), right? So maybe it would be convenient to set the default value of _PREFERENCE_LIBSC to _PREFERENCE_LIBP4EST, which means that on Linux you would not need to set both, libp4est and libsc, but setting libp4est is enough. Of course, then we should also document this properly. Any thoughts on this?

Interesting. In that case it wouldn't be a breaking change, since existing code should continue to work, or am I missing something?

[JoshuaLampert]

Yes, it wouldn't be a breaking change anymore. But on Windows you still need to set libsc to get things work including calling sc_ functions.

[ranocha]

That's a nice observation! So this will be non-breaking (it works where it worked before) but fixes problems on Windows 👍

I will go ahead and merge this now.

[lcw]

Thanks everyone!