Windows compatibility

[SebastianM-C]

Hello!
First of all, thanks for making this package (and also JuLIP).
I was wondering if this package (and by extension JuLIP) could be also installed on Windows. I looked around in the source code and the most difficult issue seems to be the matscipy dependency. If I understood correctly, this is only used for neighbor lists. From what I looked in the JuLIP source code, the julia port NeighbourLists.jl (https://github.com/JuliaMolSim/JuLIP.jl/blob/master/src/atoms.jl#L184) is used and I was wondering if the matscipy dependency could be removed.
This could greatly simplify the build process (no need for a deps/build.jl), because the python package ase could be installed automatically by switching to pyimport_conda (since ase is availabe on conda: https://anaconda.org/RMG/ase). Also, with these modifications, I think precompilation can be enabled.

In order to test this, I made the required changes on my fork, configured CI with github actions on the gh-actions branch and created a PR from a separate branch to test the changes.
As can be seen in the tests, only the parts related to the neighbourlist function fail, but the rest pass (The logs are not complete for all versions since all tests are canceled on the first failure https://github.com/SebastianM-C/ASE.jl/runs/339254643#step:5:218).
I also tested locally on Windows 10 x64, and the ase python package successfully installs and the test for ASE.jl pass (with the obvious exception of the ones related to the neighbourlist function).

If there are things left to be ported from the MatSciPy module to julia, I would like to help. If you would like, I can also make a PR for the pyimport_conda changes (and also for github actions CI and code coverage if you are interested).

[cortner]

I think this is a great suggestion - I was often wondering about removing some dependencies. Let me think through the consequences and get back to you.

[cortner]

First a bit of information: I've recently moved all my packages to the MolSim registry; if you are using JuLIP, ASE, etc from the general registry then you won't get the latest Versions anymore. See the link on how to install the MolSim registry.

But maybe you are aware of this anyhow, and if you are using master then all if fine of course.

[cortner]

Now about your suggestion: I wonder whether we could have conditional loading of the neighbourlist? If the line

matscipy_neighbours = pyimport("matscipy.neighbours")

fails, then we set a flag in the module that matscipy is unavailable. (maybe also print a warning) The Neighbourlist function could then either just throw an error or revert to the NeighbourLists.jl instead.

Another idea would be to create a new package MolSimPy.jl or similar where we collect all Python tools that we need - which could include ASE, matscipy, spglib, pymatgen etc ... This would have to be written in a way that doesn't load a Python package until it is actually needed - just to control the loading times. The more I think about it the more I like this actually.

[SebastianM-C]

Thanks for the link. I saw the MolSim registry, but I was now sure whether you will continue to also support the General registry.

Regarding the first option: the option would be a try/catch with pyimport and this would work when matscipy is already installed, or would you also like to keep the corresponding deps/build.jl?

Regarding the second: what would the status of ASE.jl becaome in this case? Will it be a pure julia package with conditional loading (via Requires.jl?) of the python bindings?

[cortner]

Regarding the first option: the option would be a try/catch with pyimport and this would work when matscipy is already installed,

exactly

or would you also like to keep the corresponding deps/build.jl?

Ideally yes. But then if it doesn't build it won't affect the usability of the package.

Regarding the second: what would the status of ASE.jl becaome in this case? Will it be a pure julia package with conditional loading (via Requires.jl?) of the python bindings?

That's the direction of my thinking - so far that's all it is, no concrete plan. Maybe as a first step, this should just remain ASE.jl and we can test this conditional loading with matscipy and expand it to other packages if needed/useful.

Just to say we'd all be thrilled to include more people in our community and working on these packages.

[SebastianM-C]

One thing that we could try is to create a MathSciPy.jl package containing the module in ASE.jl as is and use

function __init__()
    @require MathSciPy="[UUID]" include("nlist.jl")
end

[cortner]

I'm a bit cautious about creating too many packages. I know that kind of standard in the Julia eco-system, but I think it needs a good justification. I'm ok splitting ASE.jl and MolSimPy.jl but I don't think a separate package for Matscipy is justified.

[SebastianM-C]

Sorry for the confusion, I was referring to the creation of a single additional package.
What I wanted to say is that package (named MolSimPy.jl; I wrote the other name thinking of the module name) would include the Matscipy module.

[cortner]

I'd be happy with that. Then the important ASE.jl would remain separate, and very easy to use.

Why not go ahead and make a PR?

[SebastianM-C]

Here is the MolSimPy.jl pacakge: https://github.com/SebastianM-C/MolSimPy.jl
Should I transfer it to the JuliaMolSim org?

[cortner]

I invited you to become a member of JuliaMolSim Let me know if that's enough to transfer it?

[SebastianM-C]

I transferred it. Please check if everything is alright.

[SebastianM-C]

I will now proceed with the PR for ASE.jl