test solid harmonic ordering runtime switchable #271

loriab · 2023-09-04T02:01:02Z

~~currently atop #270~~

closes #225
closes #230

Downstream testing (still using `sss`, not `ss`)

Downstream testing (now `ss`-based; mostly `dev1` -> `dev2`; Oct 30)

This PR has a sample library src export
It packages on Linux/Mac/Silicon/Windows here bridge libint for oss, now v2.8.0 conda-forge/libint-feedstock#22
It has a few conda packages for testing here https://anaconda.org/psi4/libint/files . install via ~~conda install libint=2.8.0dev1 -c psi4/label/testing~~ conda install libint=2.8.0dev2 -c psi4/label/testing
It works with psi4 here try minimally changed Libint2 psi4/psi4#3047

…ion_accessor fn

loriab · 2023-10-12T23:31:38Z

This is rebased and updated and ready for consideration, I think. I'm glad to make any changes.

evaleev · 2023-10-15T23:56:37Z

INSTALL.md

+```
+   onebody_dD_lL - library includes 1-body integrals with derivative order D (D=0,1,2,...) and max angular momentum up to L (L=2,3,4,...)
+   eri_cC_dD_lL  - library includes 2-body integrals with C (C=2,3,4) centers, derivative order D (D=0,1,2,...), and max angular momentum up to L (L=2,3,4,...)
+   g12_dD-lL - library includes F12 integrals with Gaussian factors with derivative order D and max angular momentum up to L


Right, I'll change line 78 to g12_dD_lL - library includes F12 integrals with Gaussian factors with derivative order D (D=0,1,2,...) and max angular momentum up to L (L=2,3,4,...). Did I miss anything on lines 76 or 77?

evaleev · 2023-10-16T00:04:30Z

INSTALL.md

+ const char * configuration_accessor() {
+     //return "@Libint2_CONFIG_COMPONENTS@";
+-    return "(nyi)";
+    return "eri_c2_d0_l2;eri_c2_d0_l3;eri_c2_d0_l4;eri_c2_d0_l5;eri_c2_d0_l6;eri_c2_d1_l2;eri_c2_d1_l3;eri_c2_d1_l4;eri_c2_d1_l5;eri_c3_d0_l2;eri_c3_d0_l3;eri_c3_d0_l4;eri_c3_d0_l5;eri_c3_d0_l6;eri_c3_d1_l2;eri_c3_d1_l3;eri_c3_d1_l4;eri_c3_d1_l5;eri_c4_d0_l2;eri_c4_d0_l3;eri_c4_d0_l4;eri_c4_d0_l5;eri_c4_d1_l2;eri_c4_d1_l3;eri_c4_d1_l4;g12_d0_l2;g12_d0_l3;g12_d0_l4;g12_d1_l2;g12_d1_l3;g12_d1_l4;impure_sh;onebody_d0_l2;onebody_d0_l3;onebody_d0_l4;onebody_d0_l5;onebody_d0_l6;onebody_d1_l2;onebody_d1_l3;onebody_d1_l4;onebody_d1_l5;onebody_d2_l2;onebody_d2_l3;onebody_d2_l4;ss";


I am slightly concerned about the lack of support for different angular momenta on different centers in component notation ... how about instead of eri_c3_d1_l5 and eri_c2_d1_l5 we say eri_hhH_d1 and eri_HH_d1, with h and H denotting Cartesian and solid harmonic l=5 Gaussians? And instead of impure_sh we would make eri_hhh_d1 and ``eri_hh_d1` available.

It is likely Psi does not use this currently, but generally this is useful since fitting basis should include higher l than the corresponding orbital basis, so it makes sense to have "fitting" centers to support higher l than the rest...

Is the mixed AM on different centers specifiable through the build system now? (I suppose I've never specified AM for eri2 and eri3 differently.) I'm not quite following how h and H relate to generator input parameters. If h=4 and H=5, would the component be eri_hhH445_d1? Otherwise wouldn't h=4 and H=4 with pure-sh=T/F look the same?

Having two input variables (eri2 max AM and eri3 max AM) can generate a lot more components when filling in all the lesser accessible components (e.g., 445 means 444, 335, 225, etc. all accessible and might be requested by a L2 consumer), but in a pinch I can outsource it to python and not rely on CMake math.

Would 4-center remain unchanged, eri_c4_d1_l5 or eri_d1_l5, or should it switch to the new form, like eri_hhhh4444_d1?

yes, this is doable, but none of the public releases use this. Note use of lmax in https://github.com/evaleev/libint/blob/master/src/bin/libint/build_libint.cc#L1145 vs. lmax_default in https://github.com/evaleev/libint/blob/master/src/bin/libint/build_libint.cc#L1146 ... so configuring with --with-max-am=6 --with-eri3-max-am=7 would allow computing (k|ii) integrals ...

we indeed would need to provide means of "comparison" for component strings, e.g. libint2::supports("eri_ffG") ... doing this in 2 places would be a total waste of effort, might be worth building a c++ helper code and calling it from cmake to avoid logic duplication

I would prefer making format uniform for all cases, e.g. eri_hhhh_d1.

What do you think?

yes, this is doable, but none of the public releases use this. Note use of lmax in https://github.com/evaleev/libint/blob/master/src/bin/libint/build_libint.cc#L1145 vs. lmax_default in https://github.com/evaleev/libint/blob/master/src/bin/libint/build_libint.cc#L1146 ... so configuring with --with-max-am=6 --with-eri3-max-am=7 would allow computing (k|ii) integrals ...

Aha, good to know. Down the road in #259, this is going to require some changes, as I presently assume the generic max-am is simply the largest of eri-max-am, eri3-max-am, eri2-max-am, not an independent information source.

we indeed would need to provide means of "comparison" for component strings, e.g. libint2::supports("eri_ffG") ... doing this in 2 places would be a total waste of effort, might be worth building a c++ helper code and calling it from cmake to avoid logic duplication

Ok, I see what you mean now about hhH turning into ffG w/distinguishable lowercase/caps rather than 334. I'll test, but I'm pretty sure CMake component handling is case sensitive, so that'll be fine. Letters are nice for humans but a bit harder for computers to compare (though it just needs a translation string and a L=7 convention). EDIT: Now that I think on it, if components run AM together <h><h><H> rather than label them l<L>, the vars must be letters, not ints.

The c++ comparison helper code called from cmake I don't think is a great strategy, if I'm understanding it correctly. The key step is libint2-config.cmake accessed at find_package(Libint2) time. That's a fairly dumb code that works on strings and the presence/absence of files. I've never seen external functions called from it. And if there's cross-compiling happening, the helper code might not even run at detection time on detection hardware. So I think the long enumeration of accessible strings is sadly the best cmake-wise. I'd probably switch from the string being cmake-computed to it being Python-computed, adding a req'd Py to the first line build_libint https://github.com/loriab/libint/blob/new-cmake-2023-take2-b/INSTALL.md#prerequisites .

For background, my existing plan for configuration access was the following:

at build-against time consumer codes would use cmake components to find a suitable library (e.g., find_package(Libint2 COMPONENTS eri_c4_d2_l6 ss) (old notation).

at qc runtime consumer codes would call libint2::configuration_accessor() and do whatever they want with the big cmake-generated (in future) string. For instance, psi promptly turns it into a dictionary, https://github.com/psi4/psi4/blob/c6c6d7ec3509132e9bd3800af3ab87e10ed7184b/psi4/driver/p4util/util.py#L342 .

To address your libint2::supports proposal, I could have the configuration_accessor take an optional string argument, say eri_ffG. Then the fn splits at ; and returns found or not. It's a lot dumber than the comparison fn you suggested, since the combinatorics are pre-generated, but the effect to the user is the same.

I would prefer making format uniform for all cases, e.g. eri_hhhh_d1. What do you think?

Sure, I like uniform, too. I'll sketch out some input parameters to cmake component scenarios this afternoon to make sure I understand the conversion. How do you feel about retaining the explicitly enumerated component string for reasons described above?

eri_ffg_d0 (all lowercase; mixed AM) is a thing, right? That is, differing AM on the unpaired index is possible w/o --enable-eri3-pure-sh=yes. So the format string is eri_hhL_dD (always when eri3 enabled) and eri_hhl_dD (unless --enable-eri3-pure-sh)?

eri_ffg_d0 (all lowercase; mixed AM) is a thing, right? That is, differing AM on the unpaired index is possible w/o --enable-eri3-pure-sh=yes. So the format string is eri_hhL_dD (always when eri3 enabled) and eri_hhl_dD (unless --enable-eri3-pure-sh)?

exactly. hence the use of letters instead of digits

Whoops, I missed your comments from an hour ago. I'll read them next. I just pushed docs revised according to the component plan from this morning and a component list from the sample configuration.

To address your libint2::supports proposal, I could have the configuration_accessor take an optional string argument, say eri_ffG. Then the fn splits at ; and returns found or not. It's a lot dumber than the comparison fn you suggested, since the combinatorics are pre-generated, but the effect to the user is the same.

that would work, or if we are going full python for this code we can do the parse easily. The problem with enumerating all possibilities is that L^4 entries for large L can be quite large ... for heavy elements we will need full support for L=6 at least and with DF I can envision needing iiL ints (i.e. OBS max L = 6, DFBS max L = 8).

For configuration identification purposes, I was only thinking of e.g., eri_iiii_d0, eri_hhhh_d0, etc. since the different centers of a 4-center can't be specified independently in the buildsys (right?), so there's not the L^4 threat to the component list. So one wouldn't be calling find_package(Libint2 COMPONENTS eri_ghgh_d0) or libint2::supports(eri_ghgh_d0). Do you foresee people needing to call libint2::supports/configuration_acessor() with that mixed/L^4 case? I guess I figured consumers would be calling configuration_accessor once and collecting some AM/deriv limits for large-scale logic, not calling on all AM combinations.

I suppose the only immediate decision for this PR is whether you want the bool libint2::supports(std::string component) { /* pseudocode */ return component in configuration_accessor().split(";") }.

I tentatively revised the onebody_ component format for uniformity to onebody_h_dD. Not sure what to do on the g12 unless g12_hhhh_dD. I also never happened to put in components for multipole order; could be multipole_h_dD if you like.

I went ahead and added libint2::supports. Known remaining issues include settling on onebody/g12/multipole? formats and updating the sample patch. I've got a couple CHANGES changes, too.

Now with revised component code strings for onebody, g12, and multipole.

loriab · 2023-10-30T14:44:46Z

I've updated the testing of downstream packaging and psi4 usage with the revised component names. Links are in the last section of the PR frontmatter. I might add a script for the generation of cmake components for the patch (I've been doing it by hand), but that can be a separate PR if this is otherwise in good shape.

evaleev · 2023-10-30T20:17:44Z

@loriab thank you, this looks great!!! The only thing I would consider changing make the encoding of angular momenta and derivatives into non-eri components more systematic and future-proof by replacing multipole_l -> multipole_ll_dD ... i.e. all compute types can have derivatives, and for all operator set types we can have arbitrary number of centers, e.g. 4-center (1-e) overlaps are useful, although those one can get via the eri route by tweaking the core integrals. Still, would be useful to keep things systematic. What do you think?

loriab · 2023-10-31T05:17:11Z

Great! I'm all for systematizing and future-proofing the components, but I may not be following the extent of what you're proposing. Were you thinking of <operator>_[<hh>|<HH>|<hhl>|<hhL>|<hhhh>]_d<D> for each operator in https://github.com/evaleev/libint/wiki/using-modern-CPlusPlus-API#create-an-integral-engine ? E.g., eri_* get renamed to coulomb_ and g12_ to cgtg_ (plus getting 2,3,4-center components)?

On one hand, this (a) would make the connection between build configuration arguments and integrals classes provided much clearer, (b) I don't see any limitation on the length of CMake lists, (c) in future, they're all substituted into the l2-config.cmake, so there's no added complexity https://github.com/loriab/libint/blob/new-cmake-2023-take2-b/cmake/libint2-config.cmake.in#L166-L168, and (d) it's especially useful in the libint2::supports function.

On the other hand, for a max-am-eri=5,4,3 (a) there's about 100 components for eri, so that's going to be excessive multiplied by all the operators and (b) there's a lot of redundancy (e.g., --enable-g12 provides cgtg, cgtg_x_coulomb, delcgtg2).

Am I thinking about this right? I don't prefer to have them diverge, but it might be that your proposal is right for l2::supports but not for the cmake components role.

loriab · 2023-11-02T04:49:17Z

Ok, I'm still not clear on your plan for the non-eri components. But since components aren't being used at all at the moment, perhaps rolling out eri now for a trial and revisiting the full set of non-eri later is ok? To that end, I made some edits so that the present components are consistent with the future expansion.

multipole gets 2-center and deriv, so now multipole_hh_dD
onebody gets 2-center, so now onebody_hh_dD
g12 gets 4-center, so now g12_hhhh_dD

Does that seem good, or do you want me to alias the operators in l2::supplies (e.g., user queries for yukawa_hh_d0, and fn looks up eri_hh_d0)?

I've added a little generation script for the components. Input is crude, but I didn't want it too fancy since it'll be fed by cmake later on.

I also tweaked some shorderings comments in engine.h

evaleev · 2023-11-02T11:59:16Z

multipole gets 2-center and deriv, so now multipole_hh_dD

onebody gets 2-center, so now onebody_hh_dD

g12 gets 4-center, so now g12_hhhh_dD

Does that seem good, or do you want me to alias the operators in l2::supplies (e.g., user queries for yukawa_hh_d0, and fn looks up eri_hh_d0)?

probably too much ... this functionality is for experts, who presumably will read the dox. Enough to state in the dox that eri implies general Boys function-based evaluation of integrals over distance-dependent 2-particle interactions and can be used to compute Yukawa, Slater, erf-attenuated Coulomb, etc.

I've added a little generation script for the components. Input is crude, but I didn't want it too fancy since it'll be fed by cmake later on.
I also tweaked some shorderings comments in engine.h

Great, thank you!

I'll have one final lookover, but this is good to go in my opinion.

loriab · 2023-11-02T13:59:58Z

export/cmake/configuration-gen.py

+    for am in range(eri3_max_am[deriv], 1, -1):
+        for am2 in range(max_am[deriv], 1, -1):
+            if am2 >= am:
+                centers = amstr[am].lower() * 2 + amstr[am2].upper()


I need to switch these to {max_am}{max_am}{eri3_max_am}. Apparently I had it right in my head (from you) when writing INSTALL.md but switched it around by the time I wrote the script.

fixed in 3171be8

loriab

Here's a couple more typos and changes for your upcoming review. Otherwise, lgtm.

loriab · 2023-11-06T20:34:21Z

INSTALL.md

+
+#### Configuration Codes
+
+Evenually, these will be CMake Components, too.


Suggested change

Evenually, these will be CMake Components, too.

Eventually, these will be CMake Components, too.

loriab · 2023-11-08T22:22:29Z

CHANGES

@@ -4,17 +4,24 @@
 Following is a brief summary of changes made in each release of Libint.

 - 2022-xx-yy: 2.8.0-beta.1
-  - UNMERGED PR #270: For Windows, basis sets with a star have been renamed to "s" on the filesystem,
+  - UNMERGED PR #271: Add `libint2::configuration_accessor` and `libint2::supports` functions. If


Suggested change

- UNMERGED PR #271: Add `libint2::configuration_accessor` and `libint2::supports` functions. If

- PR #271: Add `libint2::configuration_accessor` and `libint2::supports` functions. If

loriab · 2023-11-08T22:23:06Z

CHANGES

-  - UNMERGED PR #270: For Windows, basis sets with a star have been renamed to "s" on the filesystem,
+  - UNMERGED PR #271: Add `libint2::configuration_accessor` and `libint2::supports` functions. If
+    library source is patched, these provides codes for what integrals a library instance can supply.
+  - UNMERGED PR #271: Small pkgconfig and cmake detection improvements. Enable unity build.


Suggested change

- UNMERGED PR #271: Small pkgconfig and cmake detection improvements. Enable unity build.

- PR #271: Small pkgconfig and cmake detection improvements. Enable unity build.

loriab · 2023-11-08T22:24:03Z

CHANGES

@@ -4,17 +4,24 @@
 Following is a brief summary of changes made in each release of Libint.

 - 2022-xx-yy: 2.8.0-beta.1
-  - UNMERGED PR #270: For Windows, basis sets with a star have been renamed to "s" on the filesystem,
+  - UNMERGED PR #271: Add `libint2::configuration_accessor` and `libint2::supports` functions. If
+    library source is patched, these provides codes for what integrals a library instance can supply.


Suggested change

library source is patched, these provides codes for what integrals a library instance can supply.

library source is patched, these provide codes for what integrals a library instance can supply.

evaleev

sorry for the delay, wanted to update the wiki but will do in another PR

evaleev · 2023-11-25T00:56:44Z

~~weird, the merge master, even though the branch built fine: 10ca429~~ nvm, code in #279 needs to be skipped (or updated) if Gaussian solid harmonic ordering is used

fixup #271

loriab force-pushed the targetsconfig branch 2 times, most recently from df6b3f8 to 204c3f2 Compare September 6, 2023 00:55

This was referenced Sep 6, 2023

CMake unconditionally installs libint2.pc into lib/pkgconfig #230

Closed

pkgconfig: libint2.7 lacks version information #225

Closed

loriab marked this pull request as ready for review September 6, 2023 13:57

loriab mentioned this pull request Sep 9, 2023

cmake harness, 2023 edition #259

Open

loriab mentioned this pull request Sep 29, 2023

adapt Libint library for Windows #270

Merged

7 tasks

loriab and others added 8 commits October 12, 2023 13:35

compile configuration_accessor into library, cmake and libtool

1f2afc3

add testing for solid harmonic ordering runtime switch and configurat…

1578fbe

…ion_accessor fn

improve exported libint2.pc file

13c01be

enable unity build. add some variables to libint2-config.cmake

e7d0720

plug memory leaks in mpf numeric functions

e39bdc2

added missing BOOST_PP_STRINGIZE in boost pp stuff in build_libint

667bf42

misc. and install notes

0934428

fix version in pkgconfig file

7df6652

loriab force-pushed the targetsconfig branch 2 times, most recently from 3183a6a to 269bc67 Compare October 12, 2023 22:08

labeling sss -> ss

55097ce

loriab force-pushed the targetsconfig branch from 269bc67 to 55097ce Compare October 12, 2023 22:10

loriab requested a review from evaleev October 12, 2023 23:31

evaleev reviewed Oct 15, 2023

View reviewed changes

evaleev reviewed Oct 16, 2023

View reviewed changes

loriab force-pushed the targetsconfig branch 2 times, most recently from 19503b8 to aacd585 Compare October 19, 2023 05:27

timostrunk mentioned this pull request Oct 26, 2023

2023.2 conda-forge/cp2k-feedstock#47

Merged

5 tasks

loriab force-pushed the targetsconfig branch 2 times, most recently from 2c75f60 to 4f37c4f Compare October 29, 2023 15:52

revised component names

fbd1d97

loriab force-pushed the targetsconfig branch from 4f37c4f to fbd1d97 Compare October 29, 2023 15:54

loriab mentioned this pull request Oct 31, 2023

python 3.12 #274

Open

further component revisions, plus script

73de7d1

loriab commented Nov 2, 2023

View reviewed changes

correct eri3 am ordering in script

3171be8

loriab commented Nov 9, 2023

View reviewed changes

loriab mentioned this pull request Nov 13, 2023

Libint2 2022 Strategy psi4/psi4#2442

Open

evaleev approved these changes Nov 24, 2023

View reviewed changes

evaleev merged commit 10ca429 into evaleev:master Nov 24, 2023
8 checks passed

loriab deleted the targetsconfig branch November 24, 2023 23:24

evaleev added a commit that referenced this pull request Nov 25, 2023

Merge pull request #285 from evaleev/evaleev/fixup/pr-271

6445d6b

fixup #271

loriab mentioned this pull request Feb 9, 2024

AM symbol list psi4/psi4#3130

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test solid harmonic ordering runtime switchable #271

test solid harmonic ordering runtime switchable #271

loriab commented Sep 4, 2023 •

edited

Loading

loriab commented Oct 12, 2023

evaleev Oct 15, 2023

loriab Oct 16, 2023

evaleev Oct 16, 2023

loriab Oct 16, 2023

evaleev Oct 16, 2023

loriab Oct 16, 2023 •

edited

Loading

loriab Oct 16, 2023

evaleev Oct 16, 2023

loriab Oct 16, 2023 •

edited

Loading

loriab Oct 17, 2023

loriab Oct 17, 2023

loriab Oct 19, 2023

loriab commented Oct 30, 2023

evaleev commented Oct 30, 2023

loriab commented Oct 31, 2023

loriab commented Nov 2, 2023

evaleev commented Nov 2, 2023

loriab Nov 2, 2023

loriab Nov 2, 2023

loriab left a comment

loriab Nov 6, 2023

loriab Nov 8, 2023

loriab Nov 8, 2023

loriab Nov 8, 2023

evaleev left a comment

evaleev commented Nov 25, 2023 •

edited

Loading


		#### Configuration Codes

		Evenually, these will be CMake Components, too.

	Evenually, these will be CMake Components, too.
	Eventually, these will be CMake Components, too.

	- UNMERGED PR #271: Add `libint2::configuration_accessor` and `libint2::supports` functions. If
	- PR #271: Add `libint2::configuration_accessor` and `libint2::supports` functions. If

	- UNMERGED PR #271: Small pkgconfig and cmake detection improvements. Enable unity build.
	- PR #271: Small pkgconfig and cmake detection improvements. Enable unity build.

	library source is patched, these provides codes for what integrals a library instance can supply.
	library source is patched, these provide codes for what integrals a library instance can supply.

test solid harmonic ordering runtime switchable #271

test solid harmonic ordering runtime switchable #271

Conversation

loriab commented Sep 4, 2023 • edited Loading

Downstream testing (still using sss, not ss)

Downstream testing (now ss-based; mostly dev1 -> dev2; Oct 30)

loriab commented Oct 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loriab Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loriab Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loriab commented Oct 30, 2023

evaleev commented Oct 30, 2023

loriab commented Oct 31, 2023

loriab commented Nov 2, 2023

evaleev commented Nov 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loriab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evaleev left a comment

Choose a reason for hiding this comment

evaleev commented Nov 25, 2023 • edited Loading

loriab commented Sep 4, 2023 •

edited

Loading

Downstream testing (still using `sss`, not `ss`)

Downstream testing (now `ss`-based; mostly `dev1` -> `dev2`; Oct 30)

loriab Oct 16, 2023 •

edited

Loading

loriab Oct 16, 2023 •

edited

Loading

evaleev commented Nov 25, 2023 •

edited

Loading