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

Jump to bottom

CompositeJK Part 3: SplitJK #2955

Merged
merged 31 commits into from Oct 2, 2023
Merged

CompositeJK Part 3: SplitJK #2955

merged 31 commits into from Oct 2, 2023

Conversation

davpoolechem
Copy link
Contributor

@davpoolechem davpoolechem commented May 8, 2023 •
edited

Description

The final step of the CompositeJK saga.

The previous PR in this chain, #2833, introduced the CompositeJK framework to Psi4, a class by which to enable arbitrary mixing and matching of different algorithms for constructing the J and K matrices separately, with front-end changes to match. The big issue with the current formulation of CompositeJK, is that every single separate J or K build algorithm in Psi4 (currently DF-DirJ, LinK, and COSX) is contained within CompositeJK itself. This runs a high risk of CompositeJK becoming monolithic, especially as more separate J/K build methods are added (e.g., CFMM).

This PR is meant to be the solution to that issue. Rather than storing every separate J or K build algorithm in CompositeJK itself, they are now implemented as derived classes of a new base class, SplitJK. SplitJK features the build_G_component member function, which derived classes of SplitJK then implement to execute their own algorithm for building one of J or K. CompositeJK is reworked to match, featuring a pair of shared_ptrs to SplitJK classes, one representing the J algorithm, and one representing the K algorithm. Actual machinery for computing J or K, as well as other functionalities such as header printing, is then done through calling corresponding functions from the SplitJK pointers.

My thought is for this PR to be added in v1.9.

User API & Changelog headlines

N/A

Dev notes & details

  • Implements the SplitJK class for representing algorithms that build one of J or K.
  • Refactors DF-DirJ, LinK, and COSX as derived classes of SplitJK.
  • Reworks the CompositeJK class to execute in terms of SplitJK derived classes.

Questions

  • Is passing the eri_computers variable to COSX::build_G_component() as an argument acceptable, even though it is unused in that function? My logic, for now, is that the SplitJK implementation allows for a universal interface for calling different SplitJK derived classes, and thus different composite algorithm combinations; but this is a case where specifically fine-tuning the input for different methods may be preferable. What does everyone think?

Checklist

Status

  • Ready for review
  • Ready for merge

@loriab loriab added this to the Psi4 1.9 milestone May 8, 2023
@loriab loriab added the scf Involves general SCF: convergence algorithms, RHF/UHF/ROHF/CUHF... label May 8, 2023
@davpoolechem davpoolechem force-pushed the dpoole34/compositejk-splitjk-squash branch 3 times, most recently from 2249f36 to 16ebc0e Compare May 15, 2023 13:52
@davpoolechem davpoolechem force-pushed the dpoole34/compositejk-splitjk-squash branch 2 times, most recently from 133d799 to 44297ec Compare May 25, 2023 14:25
davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/COSK.cc
return esp_bound;
}

COSK::COSK(std::shared_ptr<BasisSet> primary, Options& options) : SplitJK(primary, options) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::common_init() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/COSK.cc
return num_computed_shells_;
}

void COSK::print_header() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::print_COSX_header() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/COSK.cc
// build the K matrix using Neeses's Chain-of-Spheres Exchange algorithm
// algorithm is originally proposed in https://doi.org/10.1016/j.chemphys.2008.10.036
// overlap fitting is discussed in https://doi.org/10.1063/1.3646921
void COSK::build_G_component(std::vector<std::shared_ptr<Matrix>>& D, std::vector<std::shared_ptr<Matrix>>& K,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::build_COSK() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/DirectDFJ.cc

namespace psi {

DirectDFJ::DirectDFJ(std::shared_ptr<BasisSet> primary, std::shared_ptr<BasisSet> auxiliary, Options& options) : SplitJK(primary, options), auxiliary_(auxiliary) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::common_init() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/DirectDFJ.cc
return num_computed_shells_;
}

void DirectDFJ::print_header() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::print_DirectDFJ_header() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/DirectDFJ.cc

// build the J matrix using Weigend's integral-direct density fitting algorithm
// algorithm is in Figure 1 of https://doi.org/10.1039/B204199P
void DirectDFJ::build_G_component(std::vector<std::shared_ptr<Matrix>>& D, std::vector<std::shared_ptr<Matrix>>& J,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::build_DirectDFJ() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/LinK.cc

namespace psi {

LinK::LinK(std::shared_ptr<BasisSet> primary, Options& options) : SplitJK(primary, options) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::common_init() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/LinK.cc
return num_computed_shells_;
}

void LinK::print_header() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::print_linK_header() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/LinK.cc

// build the K matrix using Ochsenfelds's Linear Exchange (LinK) algorithm
// To follow this code, compare with figure 1 of DOI: 10.1063/1.476741
void LinK::build_G_component(std::vector<std::shared_ptr<Matrix>>& D, std::vector<std::shared_ptr<Matrix>>& K,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Machinery pulled from the previous CompositeJK::build_linK() function.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/jk.h
@@ -1245,32 +1250,6 @@ class PSI_API CompositeJK : public JK {
// Is the JK currently on the first SCF iteration of this SCF cycle?
bool initial_iteration_ = true;

// => Density Fitting Stuff, for Direct DF-J <= //
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to respective SplitJK derived classes.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/jk.h
@@ -1289,31 +1268,6 @@ class PSI_API CompositeJK : public JK {
/// Post-iteration Incfock processing
void incfock_postiter();

/// Build the coulomb (J) matrix using Direct DF-J
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to respective SplitJK derived classes, as build_G_component().

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/jk.h
/**
* Print header information regarding JK
* type on output file
*/
void print_header() const override;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to corresponding SplitJK derived classes, as print_header().

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
@@ -51,100 +51,6 @@ using namespace psi;

namespace psi {

Matrix compute_numeric_overlap(const DFTGrid &grid, const std::shared_ptr<BasisSet> &primary) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code block moved to libfock/COSK.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
} else {
throw PSIEXCEPTION("Invalid Composite J algorithm selected!");
}

// => Set up separate K algorithm <= //

// Linear Exchange (LinK)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code block moved to libfock/LinK.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
linK_ints_cutoff_ = cutoff_;
}

// Chain-of-Spheres Exchange (COSX)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code block moved to libfock/COSK.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
}
outfile->Printf("\n");
}
}

void CompositeJK::print_DirectDFJ_header() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function moved to DirectDFJ::print_header() in libfock/DirectDFJ.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
}
}

void CompositeJK::print_linK_header() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function moved to LinK::print_header() in libfock/LinK.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
}
}

void CompositeJK::print_COSX_header() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function moved to COSK::print_header() in libfock/COSK.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
@@ -532,1059 +291,4 @@ void CompositeJK::compute_JK() {

void CompositeJK::postiterations() {}

// build the J matrix using Weigend's integral-direct density fitting algorithm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function moved to DirectDFJ::build_G_component() function in libfock/DirectDFJ.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc

}

// build the K matrix using Ochsenfelds's Linear Exchange (LinK) algorithm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function moved to LinK::build_G_component() function in libfock/LinK.cc.

davpoolechem
davpoolechem commented May 25, 2023
View reviewed changes
psi4/src/psi4/libfock/CompositeJK.cc
// build the K matrix using Neeses's Chain-of-Spheres Exchange algorithm
// algorithm is originally proposed in https://doi.org/10.1016/j.chemphys.2008.10.036
// overlap fitting is discussed in https://doi.org/10.1063/1.3646921
void CompositeJK::build_COSK(std::vector<std::shared_ptr<Matrix>>& D, std::vector<std::shared_ptr<Matrix>>& K) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function moved to COSK::build_G_component() function in libfock/COSK.cc.

@davpoolechem davpoolechem marked this pull request as ready for review May 25, 2023 18:58
@davpoolechem
Copy link
Contributor Author

All right, I shall officially open this up for review.

@davpoolechem
Code compiles after LinK SplitJK impl
3426159
@davpoolechem
More general LinK SplitJK impl improvements
fdc142b
@davpoolechem
Start COSX SplitJK impl
1f5cbf6
@davpoolechem
More work on COSX SplitJK impl
18001dd
@davpoolechem
Fix options processing in COSX
0cc4ff7
@davpoolechem
Fix issue that popped up with scf5
44b48c7
@davpoolechem
Some updates after rebase
7313019
@davpoolechem
Add some comments removed during rebase
05783d4
@davpoolechem
Fix up compilation issues after rebase
bd155f9
@davpoolechem
Get rid of some other issues introduced by rebase
e3391cc
@davpoolechem
Retab new files, should be easier to read
0593120
@davpoolechem
Some comment improvements
4d377f4
@davpoolechem
More cleanup and header changes to SplitJK
0d8bac0
@davpoolechem
Even more straightening out of things
05116d6
@davpoolechem
Whitespace cleanup of COSK
14d73ba
@davpoolechem
Whitespace cleanup of CompositeJK.cc
8a9ad89
@davpoolechem
FIx whitespace in DirectDFJ
8e27474
@davpoolechem
Clean whitespace in LinK.cc
83589ea
@davpoolechem
Clean whitespace in SplitJK.* files
0f4b6c9
@davpoolechem
Fix previously-changed linK-1 sample
e4e03b5
@davpoolechem
Final touches, I think
fc6d728
@davpoolechem
Add print_ conditional to LinK::print_header()
ab42d24
@davpoolechem
Add correction from rebase
fe01add
@davpoolechem
Post-rebase fixups
c6b1840
@davpoolechem davpoolechem force-pushed the dpoole34/compositejk-splitjk-squash branch from 319c882 to c6b1840 Compare October 2, 2023 13:15
jturney
jturney approved these changes Oct 2, 2023
View reviewed changes
Copy link
Member

@jturney jturney left a comment

Choose a reason for hiding this comment

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

Looks good!

@davpoolechem davpoolechem added this pull request to the merge queue Oct 2, 2023
Merged via the queue into psi4:master with commit 141fcf2 Oct 2, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jturney jturney jturney approved these changes

@loriab loriab loriab approved these changes

Assignees
No one assigned
Labels
scf Involves general SCF: convergence algorithms, RHF/UHF/ROHF/CUHF...
Projects
None yet
Milestone
Psi4 1.9
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@davpoolechem @jturney @loriab