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.

Sign up for GitHub

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

test solid harmonic ordering runtime switchable #271

Merged
merged 12 commits into from
Nov 24, 2023
Merged

test solid harmonic ordering runtime switchable #271

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1f2afc3
compile configuration_accessor into library, cmake and libtool
loriab Sep 5, 2023
1578fbe
add testing for solid harmonic ordering runtime switch and configurat…
loriab Sep 5, 2023
13c01be
improve exported libint2.pc file
loriab Sep 5, 2023
e7d0720
enable unity build. add some variables to libint2-config.cmake
loriab Sep 5, 2023
e39bdc2
plug memory leaks in mpf numeric functions
evaleev Jul 6, 2021
667bf42
added missing BOOST_PP_STRINGIZE in boost pp stuff in build_libint
evaleev Jan 14, 2022
0934428
misc. and install notes
loriab Sep 5, 2023
7df6652
fix version in pkgconfig file
loriab Sep 6, 2023
55097ce
labeling sss -> ss
loriab Oct 12, 2023
fbd1d97
revised component names
loriab Oct 16, 2023
73de7d1
further component revisions, plus script
loriab Nov 2, 2023
3171be8
correct eri3 am ordering in script
loriab Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
labeling sss -> ss
  • Loading branch information
@loriab
loriab committed Oct 12, 2023
commit 55097ce0dd0209d2d8077d5d81759b7d25416562
5 changes: 3 additions & 2 deletions CHANGES
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ Following is a brief summary of changes made in each release of Libint.

- 2022-xx-yy: 2.8.0-beta.1
- UNMERGED PR #271: Small pkgconfig and cmake detection improvements. Enable unity build.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

- UNMERGED PR #270: For Windows, basis sets with a star have been renamed to "s" on the filesystem,
Testing of solid harmonics runtime switchable from #269.
- PR #270: For Windows, basis sets with a star have been renamed to "s" on the filesystem,
so 6-31g**.g94 -> 6-31gss.g94. In code, the basis can be accessed through "6-31g**" (longstanding)
or "6-31gss" (new) for all operating systems.
- UNMERGED PR #270: Adapt build system and header imports so that library and Python bindings can build on
- PR #270: Adapt build system and header imports so that library and Python bindings can build on
Windows (at least with clang-cl compiler atop MSVC). Note that a Linux- or Mac-generated export
builds on Windows; one cannot generate an export on Windows. Note also that only a static library
build, not a shared one, works on Windows (see #237).
Expand Down
38 changes: 31 additions & 7 deletions INSTALL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ libint2::finalize();
```
SHGShell: 1
SHGShell: 2
Configuration: eri_c4_d0_l2;eri_c4_d0_l3;sss;...
Configuration: eri_c4_d0_l2;eri_c4_d0_l3;ss;...
```

For the C library, a similar function is available:
Expand All @@ -26,7 +26,7 @@ For the C library, a similar function is available:
printf("CMake Configuration (C) : %s\n", configuration_accessor());
```
```
CMake Configuration (C) : eri_c4_d0_l2;eri_c4_d0_l3;sss;...
CMake Configuration (C) : eri_c4_d0_l2;eri_c4_d0_l3;ss;...
```

If you have a built libint2 library whose history you don't know, a command like this on Linux can provide the same information:
Expand All @@ -35,12 +35,11 @@ If you have a built libint2 library whose history you don't know, a command like
strings -n80 /a/random/L2/lying/around/libint2.so
```
```
eri_c2_d0_l2;eri_c2_d0_l3;eri_c2_d1_l2;eri_c3_d0_l2;eri_c3_d0_l3;eri_c3_d1_l2;eri_c4_d0_l2;eri_c4_d1_l2;impure_sh;onebody_d0_l2;onebody_d0_l3;onebody_d1_l2;sss
eri_c2_d0_l2;eri_c2_d0_l3;eri_c2_d1_l2;eri_c3_d0_l2;eri_c3_d0_l3;eri_c3_d1_l2;eri_c4_d0_l2;eri_c4_d1_l2;impure_sh;onebody_d0_l2;onebody_d0_l3;onebody_d1_l2;ss
```

A patch like the following is suitable for an export tarball generated from the next following. See
https://github.com/loriab/libint/blob/new-cmake-2023-take2-b/cmake/libint2-config.cmake.in#L37-L65
for decoding the configuration components.
A patch like the following is suitable for an export tarball generated from the next following.
[See guide](#configuration-codes) for decoding the configuration components.

```
--- src/configuration.cc.cmake.in 2023-09-05 09:13:50.000000000 -0400
Expand All @@ -50,7 +49,7 @@ for decoding the configuration components.
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;sss";
+ 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";
Copy link
Owner

Choose a reason for hiding this comment

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

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...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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?

Copy link
Owner

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

@loriab loriab Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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:

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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)?

Copy link
Owner

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

@loriab loriab Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

}
```
```
Expand All @@ -68,3 +67,28 @@ for decoding the configuration components.
--with-eri2-max-am=6,5 \
--with-max-am=6,5
```

#### Configuration Codes

Evenually, these will be CMake Components, too.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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


```
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
Copy link
Owner

Choose a reason for hiding this comment

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

typo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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?


impure_sh - library doesn't assume that 2- and 3-center integrals involve pure solid harmonics

cart shell_set used_by
-------- --------- -------
ss - library integrals use ordering standard + standard = mpqc4, cp2k, psi4 (psi4 requires runtime-setting of solid harmonic ordering to Gaussian)
so - library integrals use ordering + orca
is - library integrals use ordering intv3 + standard = mpqc3
io - library integrals use ordering + orca
gs - library integrals use ordering gamess + standard = gamess
go - library integrals use ordering + orca
os - library integrals use ordering orca + standard
oo - library integrals use ordering + orca = orca
bs - library integrals use ordering bagel + standard = bagel
bo - library integrals use ordering + orca
```
1 change: 1 addition & 0 deletions export/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ exportdir::
-$(INSTALL) $(INSTALLLIBOPT) $(SRCDIR)/INSTALL.export $(TOPDIR)/$(EXPORTDIR)/INSTALL
-$(INSTALL) $(INSTALLLIBOPT) $(SRCDIR)/LICENSE.export $(TOPDIR)/$(EXPORTDIR)/LICENSE
-$(INSTALL) $(INSTALLLIBOPT) $(SRCTOPDIR)/README.md $(TOPDIR)/$(EXPORTDIR)/README.md
-$(INSTALL) $(INSTALLLIBOPT) $(SRCTOPDIR)/INSTALL.md $(TOPDIR)/$(EXPORTDIR)/INSTALL.md
-$(INSTALL) $(INSTALLLIBOPT) $(SRCTOPDIR)/COPYING $(TOPDIR)/$(EXPORTDIR)/COPYING
-$(INSTALL) $(INSTALLLIBOPT) $(SRCTOPDIR)/COPYING.LESSER $(TOPDIR)/$(EXPORTDIR)/COPYING.LESSER
-$(INSTALL) $(INSTALLLIBOPT) $(SRCTOPDIR)/CITATION $(TOPDIR)/$(EXPORTDIR)/CITATION
Expand Down
24 changes: 13 additions & 11 deletions tests/hartree-fock/hartree-fock++-validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,22 @@ def validate(label, data, refdata, tolerance, textline):
-8.04716669024124, 0.638204907990955, -6.35134519242917 ]
Qtol = 1e-8

# Note that the sphemultipole order is fixed at generator build time (not influenced by libint2::solid_harmonics_ordering())
if (LIBINT_SHGSHELL_ORDERING == LIBINT_SHGSHELL_ORDERING_STANDARD):
smuref = [-muref[1]/2, muref[2], -muref[0]/2] # [ 0.046, -0.105, 0.132]
print("Checking sphemultipole LIBINT_SHGSHELL_ORDERING=Standard\n");
else:
smuref = [ muref[2], -muref[0]/2, -muref[1]/2] # [-0.105, 0.132, 0.046]
print("Checking sphemultipole LIBINT_SHGSHELL_ORDERING=Gaussian\n");
# * Note that the sphemultipole order is fixed at generator build time (not influenced by
# libint2::solid_harmonics_ordering())
# * As of https://github.com/evaleev/libint/commit/cdbb9f3, sphemultipole is hard-coded at Standard
# (and not influenced by LIBINT_SHGSHELL_ORDERING)
smuref = [-muref[1]/2, muref[2], -muref[0]/2] # [ 0.046, -0.105, 0.132]
smutol = 1e-9

if (LIBINT_SHGSHELL_ORDERING == LIBINT_SHGSHELL_ORDERING_STANDARD):
sQref = [Qref[1]/4, -Qref[4]/2, (2*Qref[5] - Qref[0] - Qref[3])/4, -Qref[2]/2, (Qref[0] - Qref[3])/8 ]
else:
sQref = [(2*Qref[5] - Qref[0] - Qref[3])/4, -Qref[2]/2, -Qref[4]/2, (Qref[0] - Qref[3])/8, Qref[1]/4]
sQref = [Qref[1]/4, -Qref[4]/2, (2*Qref[5] - Qref[0] - Qref[3])/4,
-Qref[2]/2, (Qref[0] - Qref[3])/8 ]
sQtol = 1e-8
print("Checking sphemultipole Standard\n");

# Reference values from LIBINT_SHGSHELL_ORDERING=Gaussian. These are no longer accessible after cdbb9f3.
# print("Checking sphemultipole Gaussian\n");
# smuref = [ muref[2], -muref[0]/2, -muref[1]/2] # [-0.105, 0.132, 0.046]
# sQref = [(2*Qref[5] - Qref[0] - Qref[3])/4, -Qref[2]/2, -Qref[4]/2, (Qref[0] - Qref[3])/8, Qref[1]/4]

F1ref = [-5.43569555903312, -1.88298017654395, -2.17427822361352,
3.47022732536532, -2.96798871167808, 2.59189820350226,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_g.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2018-2020 Edward F. Valeev
* Copyright (C) 2018-2023 Edward F. Valeev
*
* This file is part of Libint.
*
Expand Down