Density Screening Refactor Part 2: Implementation of shell_significant()

[davpoolechem]

Description

This PR is the second in a series of planned PRs designed to remove density screening from the TwoBodyAOInt object and into the JK object, with PR #2547 being the first such PR. Having density screening available in TwoBodyAOInt runs the risk of applying density screening to algorithms where density screening doesn't make sense. Thus, it would be a good idea to move the logic of density screening to where it is more correctly applied, i.e., the JK object.

The primary purposes of this PR are twofold:

1. First, this PR introduce the shell_significant() framework to the JK class. The shell_significant() framework starts with a shell_significant() virtual function that exists in the base JK class. The shell_significant() function can then be redefined specifically for different JK derived classes as needed. The existence of shell_significant() provides a unified framework for performing screening for any JK method, and it also provides the method by which density screening can be added directly to the relevant JK classes.
2. Second, this PR uses the shell_significant() framework to reimplement screening for certain JK methods. Most significantly, as implied in the first point, the biggest change in this regard was the removal of shell_significant_density() from TwoBodyAOInt into the domain and its reimplementation into the DirectJK shell_significant() definition. With this change, density screening is now the domain of individual JK classes rather than the TwoBodyAOInt class, which was one of the primary goals of this overall refactor PR set in the first place. Additionally, another method that was subject to slight changes from the shell_significant() framework was the Yoshimine PKJK algorithm, in which its test for shell significance was implemented using the shell_significant() framework.

Notes

I wanted to outline what the next steps were for this chain of PRs, since this one accomplishes one of the main goals of the refactor in the first place:

1. Separate out density screening from the SCREENING keyword and into its own keyword (likely something like DENSITY_SCREENING)
2. Removal of any density matrix function/variable from TwoBodyAOInt. Without density screening in TwoBodyAOInt, these density matrix references in TwoBodyAOInt are unnecessary and more properly placed into JK, as well.

Todos

• [x] Implementation of shell_significant() framework in JK class to represent shell significance testing.
• [x] Removal of shell_significant_density() from TwoBodyAOInt.
• [x] Reimplementation of density screening in DirectJK via the shell_significant framework.
• [ ] Reimplementation of density screening in other JK methods via the shell_significant framework.
• [x] Alteration of other shell quartet screening implementations in other JK algorithms via the JK framework.

Questions

• [ ] The shell_significant framework has not yet been added to DFJCOSK. Should that be done this PR, or added in a later PR?

Checklist

• [x] Tests added for any new features
• [x] All or relevant fraction of full tests run

Status

• [x] Ready for review
• [ ] Ready for merge

[davpoolechem]

Looks good, I left some comments. Namely, my biggest concern is about removing the screening density keyword and moving update_density away from TwoBodyAOInt. I think it would make more sense to get it all done in one PR, but that is my personal opinion.

Thank you very much for the comments! I do appreciate it. As for your points, my thought was to implement both of these changes in future PRs, maybe as a single future PR, maybe as their own separate ones. I think there's a good argument to be made for making a separate option for density screening in this PR, and I'd be quite willing to do that. But, I do believe that removing update_density should be its own PR, as that PR would consist of removing ALL references to the density matrix from TwoBodyAOInt, and I think it would be easier to review and clearer intent if update_density was separated out in its own PR.

[loriab]

undefined symbol: _ZNK3psi12TwoBodyAOInt22shell_pair_max_densityEiii in case you hadn't clicked through to it yet

[davpoolechem]

undefined symbol: _ZNK3psi12TwoBodyAOInt22shell_pair_max_densityEiii in case you hadn't clicked through to it yet

I did indeed notice that! I think I know the problem, so I will apply the fix in just a little bit.

[davpoolechem]

All right, CI issue should be fixed now!

[JonathonMisiewicz]

FTR, David and I have agreed to pause this PR until after #2682 and #2665 come in.

[davpoolechem]

Blocking this PR until we can resolve the question of "is TwoBodyAOInt responsible for sieving, or is ERISieve"? Happy to talk about this after ACS Chicago.

To add an update to this, some of us (Jonathan, I, and others) discussed this issue at PsiCon 2022. We came to the conclusion that TwoBodyAOInt would be responsible for sieving, and that ERISieve will be removed from Psi4. As use of ERISieve is seemingly localized to the PKJK algorithm, I will be the one responsible for its removal.

Until then, this PR will likely be further delayed.

[andyj10224]

@davpoolechem Since #3098 has been opened, should we close this PR?