gridap/MiniQhull.jl

Use Binarybuilder

SimonDanisch opened this issue · 6 comments

Nice package :)
It should be pretty straightforward to pre build the binaries with BinaryBuilder:
https://github.com/JuliaPackaging/BinaryBuilder.jl

https://github.com/JuliaPackaging/Yggdrasil

Glad that you like the package!

Yes, this could be done. We haven't done it yet since we have no experience with BinaryBuilder.

Is anybody from the community willing to help us with BinaryBuilder?

I've made a PR to build Qhull here: JuliaPackaging/Yggdrasil#688

Once that's done, we could either deprecate the wrapper, move it to BinaryBuilder as well, or build it within Qhull. Is there any particular reason a C wrapper is needed - could we do the wrapping within Julia?

Hi @asinghvi17 thanks the info!

We are using a C wrapper in order to deal with some pre-processor macros (e.g, FORALLfacets ) that are defined in Qhull and needed to iterate over the computed triangulation to recover the cell connectivities.

In particular, we don't know how to write this C code in julia with ccall:

  FORALLfacets
    {
      if (! facet->upperdelaunay) (*numcells)++;
      if (! facet->simplicial) return -1;
    }

https://github.com/gridap/MiniQhull.jl/blob/master/deps/MiniQhullWrapper/src/lib/qhullwrapper.c

Do you think it is feasible to remove the wrapper and write this code in julia?

I just took a look at the FORALL* macros - they essentially loop over some field of the qh struct which is presumed to be available in the function. Unfortunately, replicating this in Julia would reqire us to fully wrap Qhull. That might be a good direction to go in, though, since Qhull is a pretty versatile library.

Dear all, thanks for your insights.

In this PR #6 we start taking advantage of Qhull_jll artifact (if julia >= 1.3) to avoid manual Qhull instalation.

MiniQhull goal is to simply expose the delaunay function. Completely wrap the Qhull interface is out-of-scope, so we will continue using an intermediate C layer to keep it as simple as possible.

I'm going to close this PR.

Please, feel free to reopen it if needed