FermiQC / Fermi.jl

Fermi quantum chemistry program
MIT License
134 stars 24 forks source link

Drop `deps/build.jl` #113

Closed carstenbauer closed 2 years ago

carstenbauer commented 2 years ago

libcint: replace the custom cmake build step with libcint_jll (see #111, solved by PR #112)

deps/lib.gz: this seems to be a data dependency so a few things come to mind:

@gustavojra where does the lib.gz come from? Do you compile it manually? Or do you download it from an external URL?

gustavojra commented 2 years ago

lib.gz is just a collection of basis set files (e.g. sto-3g.gbs) compressed. It comes from Basis Set Exchange (BSE). The original idea was to look up and download basis as needed, or just try to download basis if they are not saved locally. But, as you can see, that never happened.

I think it is important to easily allow the introduction of new basis set files.

The file parsing right now is a little "brute force", so it could be nice to have something like

basis["sto-3g", "O"]

But I don't think it is crucial.

What is the issue you see with the compressed file? The need for unpacking (and therefore a specific package for it)?

carstenbauer commented 2 years ago

What is the issue you see with the compressed file? The need for unpacking (and therefore a specific package for it)?

Yes, mostly the unpacking part. Generally, Julia packages should be "immutable snapshots". The legacy deps/build.jl approach almost inevitably undermines this idea since it mutates the package directory. Here, it downloads libcint, compiles it, and unpacks lib.gz.

But apart from this "conceptual" point of view, the deps/build.jl solution also has very practical disadvantages:

gustavojra commented 2 years ago

Thank you. For now I will ship the gbs files inside a folder to bypass the extraction and libcint_jl will be use as default. I have merged your PR.

As I said here https://github.com/FermiQC/Fermi.jl/issues/111#issuecomment-1006847621. We plan to use GaussianBasis.jl as the "integral library" and Fermi will deal with methods only/mostly. So I will copy the modifications made here into that package.

gustavojra commented 2 years ago

112