Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fundamental changes to GaussianBasis: working with different backends #10

Merged
merged 8 commits into from
Jul 13, 2022

Conversation

gustavojra
Copy link
Member

As Yall know we have two ways to compute integrals:

  • Using the fast and efficient C library libcint

  • Using the nice and slow Julia code now named acsint

To make it easier to work with different backends and open up the possibility of adding new ones in the future I modified the basis set and basis functions objects.

Basis Functions

Basis functions now can be two types

  • CartesianShell
  • SphericalShell

The fundamental difference being, as you guessed, how angular momentum is treated.

These two concrete types are a subtype of BasisFunction. The default is SphericallShell. If you do something like

BasisFunction(args...)

A SphericalShell is created. To create a cartesian, use the keyword argument spherical=false.

Another important change is a new field for the BasisFunction object called atom. That means that now the basis function objects are attached to an atom, not floating around with no center like before. This removes the necessity of having a list of lists representing basis set... which leads us to...

BasisSet

There are mainly two changes to the basis set object

  1. the basis field is now just a flat array (Vector) of basis functions. The mapping to atoms is less important now that the field atom has been introduced to BasisFunction.

  2. Those lc_atm, lc_env fields that were specific to libcint are now removed. Instead, there is a new struct called IntLib that will determine which backend is used. So now we have

struct ACSint <: IntLib end
# struct flag for computations using Libcint.jl
struct LCint <: IntLib 
    atm::Vector{Cint}
    natm::Cint
    bas::Vector{Cint}
    nbas::Cint
    env::Vector{Cdouble}
end

Notice that all information relevant to libcint is saved into this new struct, meaning that if we decide to work with ACSint (like Henry and I are) we don't need to worry about those objects. This is important because the data here must be very stype stable otherwise we get those bloody segfaults from a C-call.

BasisSet objects now have the following type signature:

BasisSet{L<:IntLib, A<:Atom, B<:BasisFunction}

So you can define a general (fall-back) function

func(B::BasisSet)

and a specific one

func(B::BasisSet{LCint})

that will use libcint.

Integral backends

Standard function call

I create a standard syntax to call integrals. Using overlap as an example you can make the following function calls on a BasisSet object

# Create the full matrix
overlap(BS::Basisfunction)
# write the matrix to out
overlap!(out, BS::BasisFunction)
# Give a specific shell pair integral
overlap!(out, BS::BasisFunction, i, j)

This last function call is the most basic one and, in general, is the one the will select the backend library based on the BasisSet type signature. For example, for the ERI we have

function ERI_2e4c!(out, BS::BasisSet{LCint}, i, j, k, l)
    cint2e_sph!(out, [i,j,k,l], BS.lib) # This is just a wrap around the C-call, check the `Libcint.jl` file if curious
end

function ERI_2e4c!(out, BS::BasisSet, i, j, k, l)
    generate_ERI_quartet!(out, BS, i, j, k, l)   # This is the julia function in `Acsint.jl` that actually computs the integral 
end

what now

Since this may affect yall's projects, I want you to look through it before we merge. Leave and questions here and Slack me if needed. Make sure you understand how to get your integrals under this system and also how would you go about coding up a new integral call!

…nding on how the basis set object is constructed. By default, basis sets proper for Libcint are created, but ACSint will be the fall back strategy for when the code cant find a specific dispatch.
Copy link
Member

@sgoodlett sgoodlett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming you will fix the CI failures, this looks good. Nate has done more with GaussianBasis and so he will probably have more to say than I do.

@nlk36701
Copy link

nlk36701 commented Jul 6, 2022

These changes won't have a drastic effect on the SALC code. I have no concerns!

@nlk36701 nlk36701 self-requested a review July 6, 2022 14:48
@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #10 (67d01dc) into main (1ec7ad2) will increase coverage by 0.25%.
The diff coverage is 86.64%.

@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
+ Coverage   80.67%   80.92%   +0.25%     
==========================================
  Files          14       16       +2     
  Lines        1164     1499     +335     
==========================================
+ Hits          939     1213     +274     
- Misses        225      286      +61     
Impacted Files Coverage Δ
src/Integrals/Multipole.jl 24.32% <31.81%> (ø)
src/Misc.jl 52.00% <52.00%> (ø)
src/GaussianBasis.jl 72.72% <66.66%> (-2.28%) ⬇️
src/Integrals/OneElectron.jl 74.62% <67.92%> (-23.79%) ⬇️
src/Integrals/TwoElectronFourCenter.jl 96.77% <86.36%> (-2.35%) ⬇️
src/BasisSet.jl 89.47% <90.32%> (+9.14%) ⬆️
src/Libcint.jl 82.14% <90.90%> (-0.47%) ⬇️
src/Acsint.jl 95.95% <95.95%> (ø)
src/Gradients/OneElectronGrad.jl 95.41% <97.40%> (+1.09%) ⬆️
src/BasisParser.jl 94.05% <100.00%> (+3.25%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46957bc...67d01dc. Read the comment docs.

@sgoodlett
Copy link
Member

Not a fan of the lapse in CodeCov...

@gustavojra
Copy link
Member Author

gustavojra commented Jul 7, 2022

Not a fan of the lapse in CodeCov...

I will fix it

@gustavojra gustavojra merged commit c474131 into FermiQC:main Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants