Make PyCall an optional dependency

[mfherbst]

I am currently experimenting with moving to PythonCall instead of using PyCall for python dependencies. It seems to have a few advantages (especially with respect to managing python dependencies from Julia), so I'm giving it a try. One unfortunate issue is, however, that using both PythonCall and PyCall in the same project can lead to issues (in the sense of a dying Julia session). For that reason I am wondering if there is an easy way to make Brillouin independent of requiring PyCall as a dependency. It seems to only be used in one submodule. How much effort would this be @thchr ?

[thchr]

It would indeed be very nice to do away with the PyCall dependency. It's used to get access to Qhull bindings via SciPy (for Wigner-Seitz cells). We use Voronoi and ConvexHull from Qhull.

Unfortunately, the only non-python-wrapper of Qhull in Julia, MiniQHull.jl, doesn't expose that functionality, as far as I can see - and also doesn't seem to work in Mac/Win (gridap/MiniQhull.jl#5). Qhull.jl just wraps SciPy unfortunately (JuliaPolyhedra/QHull.jl#19), so that doesn't help.

There was almost a full wrapper of the Qhull library in DirectQHull.jl, but it stalled because the Qhull developer is not active (specifically, qhull/qhull#105 is needed).

One pragmatic solution could be to simply use PythonCall.jl instead: I understand that's the suggested solution going forward right?

[thchr]

Revisiting this, I'm pretty sure we could just use DirectQHull.jl immediately if qhull/qhull#105 was merged and Qhull_jll then updated (and DirectQHull.jl possibly registered). So close, yet somehow not there.

[mfherbst]

Thanks @thchr for looking into this. I see, but unfortunately qhull development seems to have stalled. I don't see that it's likely this will be merged soon.

On the python side, yes it seems PythonCall is the future, but I'm personally still experimenting and I have not yet made up my final opinion. I think it's similar for many and moreover PyCall is pretty widespread. Given there can be issues in trying to get both of them to work together, I think short-term the best solution is to avoid any hard dependency on one or the other to the largest extent possible.

For example on my side, even switching to PythonCall will not magically solve the problem as DFTK still has a PyCall dependency ... and removing that will introduce some breaking changes, so that's going to happen only slowly.

[thchr]

Unfortunately, I don't think we can just excise the Qhull functionality from Brillouin (me and students I work with depend on the wignerseitz(...) functionality to visualize the Brillouin zone and the path in it), so the solution probably has to include a way to call Qhull.

In principle, the KPath submodule could be factored out into a subpackage - the reason I originally split this package up into submodules was because I was unhappy with the PyCall dependency and pondered splitting the package - but it seems like more effort and maintenance headache than it's worth.

[mfherbst]

I see thanks for the clarification.

Ok, maybe a concerted motion to PythonCall could be the easiest solution after all? But of course I don't want to impose our requirements on you @thchr and the users of Brillouin.jl.

[thchr]

@brainandforce let me know about this reference https://www.ams.org/publicoutreach/feature-column/fc-2013-11 which describes an approach to getting the Wigner-Seitz cell without depending on the full machinery of convex hulls/Voronoi cells (by exploiting that Wigner-Seitz cells are special in the sense that they are Voronoi cells that can tile space).

Seems like it would be a fair bit of work to implement this approach - but maybe some day if there's a need for a rabbit hole...

[brainandforce]

In principle, it should be much easier to take a matrix of lattice basis vectors as an argument for a function that returns the Wigner-Seitz cell of that lattice. The issues I see in this are:

• Reduction of the lattice basis to a standard form. The resource I linked suggests that we could use a QR decomposition to rotate the basis into a standard position, then find a suitable unimodular matrix U so that the columns of RU are minimized (with off-diagonal entries the smallest negative numbers possible) to get the superbasis. I know other algorithms exist for lattice basis reduction (Minkowski, Lenstra-Lenstra-Lovász, etc.) so perhaps I'm reinventing the wheel here.
• Representing the Wigner-Seitz cell of the lattice to the Cell type.

I think this might be suited for its own package (perhaps a standalone WignerSeitz.jl).

[thchr]

Thanks to some recent merges in the qhull repo and the really nice wrapper by @JuhaHeiskala, this is now fixed by depending on DirectQhull.jl for access to qhull 🚀 !

[mfherbst]

Awesome, thanks @thchr and @JuhaHeiskala!