wavefunction91 / IntegratorXX

Reusable DFT Grids for the Masses
BSD 3-Clause "New" or "Revised" License
13 stars 9 forks source link

Various improvements to Gauss-Chebyshev equations #34

Closed susilehtola closed 1 year ago

susilehtola commented 1 year ago

Per @loriab comment in #27, gausscheby1 had a wrong equation in the code. This is fixed by this PR.

This PR also converts all gausscheby routines to use the same indexing. In my opinion, it is best to choose the indexing such that the equations match the mathematical expressions, since this makes checking their correctness easier.

The third thing this PR does is to make sure that all Gauss-Chebyshev quadratures generate nodes in ascending order.

(The correctness bugs were already fixed in #32.)

loriab commented 1 year ago

for abcissas for the third rule at https://github.com/wavefunction91/IntegratorXX/pull/27#issuecomment-1625193283 you have cos^2 whereas here in the code you have plain cos. Otherwise all three lgtm. I like the clearer indexing.

wavefunction91 commented 1 year ago

+1 for the clearer indexing, but the bugs were fixed in #32. I'll address @susilehtola 's comments there and get that merged, and we can address the indexing with this PR

loriab commented 1 year ago

I don't know if you're needing a comment for the Psi4 perspective, but I'm not following what ascending and descending refer to.

wavefunction91 commented 1 year ago

@loriab As in having the quadratures stored in increasing (ascending) or decreasing (descending) order - Psi4 assumes the latter, whereas I prefer to work with the former

The defaulting ordering is descending:

for(int i = 1; i < n; ++i) {
  auto xi = cos( M_PI * i / (n+1) ); // x[i] > x[i+1] for i in [0,n)
}

For GC-{1,2}, you can fix the order by just swapping the sign since the quadratures are symmetric. For GC-2-mod and GC-3, it's not so clear due to the change in variables.

susilehtola commented 1 year ago

Rebased on top of #32. Indexing is now consistent, and all rules generate nodes in ascending order. I also added a test for the correct ordering; it turns out that all the rules gave points in the opposite order.

wavefunction91 commented 1 year ago

@susilehtola Update with master (there's a minor conflict) and this is g2g.