IncFock Standardization Part 3: DFJCOSK #2816

davpoolechem · 2022-12-01T16:21:50Z

Description

... and another one!

This PR is a continuation of the IncFock standardization effort initially started in #2682, and continued in #2792 and #2808.

This PR continues to standardize the Incremental Fock process across the current integral-direct JK algorithms present in Psi4. However, this PR stands somewhat in contrast to #2682 and #2792. In those PRs, their IncFock schemes were adapted to use that of DFJCOSK, without removing the bells and whistles of their IncFock implementations (e.g., recomputing the full Fock matrix every so often, disabling IncFock after a given convergence threshold). In contrast, DFJCOSK is the template IncFock upon which the two previous PRs were based; however, DFJCOSK does not have the IncFock bells and whistles that DirectJK and DFJLinK had. Unmitigated, the incremental Fock procedure can actually significantly increase the number of SCF iterations needed to converged; and the IncFock bells and whistles in DirectJK and DFJLinK notably mitigate the effect of IncFock on SCF convergence counts. Without these bells and whistles, DFJCOSK runs the significant risk of excessive SCF iterations needed to converge when IncFock is enabled, an issue I have run into in my own calculations.

This PR seeks to rectify the aforementioned issue by implementing the IncFock bells and whistles of DirectJK and DFJLinK into DFJCOSK. With these extra features, DFJCOSK can now recompute the full Fock matrix every n iterations, or disable IncFock past a given convergence threshold, at the will of the end user. These features can notably reduce the amount of SCF iterations needed to converge.

This PR is also the continued effort to standardize IncFock among all integral-direct SCF algorithms in Psi4. With this PR, DirectJK, DFJLinK, and DFJCOSK will have effectively the exact same incremental Fock schemes, each featuring the lower memory usage of DFJCOSK's former IncFock scheme, and the bells and whistles of DirectJK and DFJLinK's former IncFock schemes. The final step to the IncFock standardization process, then, is the movement of the IncFock scheme of these algorithms into the base JK class itself - the final step of the PR scheme discussed in the comments in #2682.

It is worth noting for the reviewers that, since this PR more closely matches the IncFock of DFJCOSK to that of DFJLinK, it should be considered relevant to the CompositeJK PR chain as well, as it smooths the combination of DFJLinK and DFJCOSK into a single class as planned in the next CompositeJK step.

User API & Changelog headlines

DFJCOSK now uses the "INCFOCK" keyword to control usage of the incremental Fock procedure, rather than "COSX_INCFOCK", standardizing the incremental Fock keywords among the integral-direct SCF algorithms.
DFJCOSK can now recompute the Fock matrix every n iterations during a calculation using incremental Fock. The "INCFOCK_FULL_FOCK_EVERY" keyword controls the number of SCF iterations between full Fock matrix recomputations for DFJCOSK.
DFJCOSK can now disable the incremental Fock process when a specific SCF convergence threshold. The "INCFOCK_CONVERGENCE" keyword controls the SCF convergence threshold point at which the incremental Fock build process is disabled.

Dev notes & details

Improves the DFJCOSK incremental Fock procedure by allowing it to recompute the full Fock matrix every n iterations.
Improves the DFJCOSK incremental Fock procedure by allowing it to disable the incremental Fock procedure past a certain SCF convergence threshold.
Improves the DFJCOSK incremental Fock prodecure by computing the full Fock matrix during the first SCF iteration of the calculation, even with INCFOCK enabled.
Adds incremental Fock build efficiency tests to test_dfjcosk.py.
Updates documentation to reflect new changes.

Questions

N/A

Checklist

Tests added for any new features
All or relevant fraction of full tests run

Status

Ready for review
Ready for merge

davpoolechem · 2022-12-01T18:00:54Z

So the CI issue on this one seems... interesting, as well.

2022-12-01T16:23:23.8944651Z Err:1 http://azure.archive.ubuntu.com/ubuntu focal/main amd64 libjsoncpp1 amd64 1.7.4-3.1ubuntu2 2022-12-01T16:23:23.8945670Z 503 Service Unavailable [IP: 52.147.219.192 80] 2022-12-01T16:23:28.9307925Z Get:2 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 libllvm6.0 amd64 1:6.0.1-14 [15.2 MB] 2022-12-01T16:29:39.8525876Z Get:3 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 libclang-common-6.0-dev amd64 1:6.0.1-14 [3015 kB] 2022-12-01T16:30:07.1867680Z Get:4 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 libclang1-6.0 amd64 1:6.0.1-14 [7472 kB] 2022-12-01T16:31:01.2307594Z Get:5 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 clang-6.0 amd64 1:6.0.1-14 [9831 kB] 2022-12-01T16:32:37.6678852Z Get:6 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 libomp5-10 amd64 1:10.0.0-4ubuntu1 [300 kB] 2022-12-01T16:32:38.5410720Z Get:7 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 libomp-10-dev amd64 1:10.0.0-4ubuntu1 [47.7 kB] 2022-12-01T16:32:38.5769178Z Get:8 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 llvm-6.0-runtime amd64 1:6.0.1-14 [207 kB] 2022-12-01T16:32:38.6670727Z Get:9 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 llvm-6.0 amd64 1:6.0.1-14 [4889 kB] 2022-12-01T16:33:14.3718902Z Get:10 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 llvm-6.0-dev amd64 1:6.0.1-14 [24.0 MB] 2022-12-01T16:35:33.4054716Z Get:11 http://azure.archive.ubuntu.com/ubuntu focal/universe amd64 libomp-dev amd64 1:10.0-50~exp1 [2824 B] 2022-12-01T16:35:33.4069243Z Fetched 65.0 MB in 12min 10s (89.1 kB/s) 2022-12-01T16:35:33.4339396Z E: Failed to fetch http://azure.archive.ubuntu.com/ubuntu/pool/main/libj/libjsoncpp/libjsoncpp1_1.7.4-3.1ubuntu2_amd64.deb 503 Service Unavailable [IP: 52.147.219.192 80] 2022-12-01T16:35:33.4341544Z E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing? 2022-12-01T16:35:33.4444758Z ##[error]Bash exited with code '100'. 2022-12-01T16:35:33.4494014Z ##[section]Finishing: Apt-Get Packages 2022-12-01T16:35:33.6248939Z ##[section]Starting: Checkout psi4/psi4@refs/pull/2816/merge to s 2022-12-01T16:35:33.6261343Z

Seems to be an issue with loading and installing packages with apt-get? Maybe this is a one-time thing that can be fixed with a CI restart.

psi4/src/psi4/libfock/DFJCOSK.cc

loriab · 2022-12-02T21:49:47Z

psi4/src/psi4/libfock/jk.h


+    // Is the JK currently on a guess iteration


It could be useful to expand on this definition. I had supposed it was simply not-the-SAD iteration, but can it be initial more than once in a jobs? if there's a guess from a smaller basis? if there's DF iterations before CONV direct iterations?

I dug into this a little bit. For DF_SCF_GUESS=True runs, it shows as true on the first CONV direct SCF iteration and false otherwise. For runs using a BASIS_GUESS, it shows up as true for the first iteration of each SCF cycle present (i.e., once for both the smaller and the larger basis set SCFs) and false otherwise. This is all from my testing.

Further looking into it, this is a feature of the old DirectJK/DFJLinK IncFock scheme that does two things. First, that disables incremental Fock construction on the first SCF iteration of a given cycle. Second, it is set to true when the size of the previous D matrix vector is not the same length as the current D matrix vector. The second component is described in a comment in the original IncFock scheme in DirectJK as being "used to handle stability analysis case". I couldn't tell you how or why that particular component works, but I carried it over to make sure nothing broke.

Regardless, you're right; the comment could use a better description. I'm thinking something like // Is the JK currently on the first SCF iteration of this SCF cycle?

psi4/src/read_options.cc

loriab · 2022-12-02T22:03:05Z

tests/pytests/test_dfjcosk.py

+
+    # how do SCF iteration counts compare?
+    if hasattr(wfn_dfjcosk_noinc, "iteration_") and hasattr(wfn_dfjcosk_inc, "iteration_"):
+        assert compare_values(wfn_dfjcosk_noinc.iteration_, wfn_dfjcosk_inc.iteration_, 3)


niter_noinc = int(wfn_dfjcosk_noinc.variable("SCF ITERATIONS")) can give you the count, too

I'm not sure what you're doing in the assert. why would the the niter be the same to 1.e-3? You could do assert compare(True, (niter_noinc - niter_inc) >= 3, "reduced by at least 3 iterations by incfock") if that's what you had in mind.

What I was trying to do with the test was to make sure that the SCF iteration count for the IncFock run was not significantly higher than that of the non-IncFock run. IncFock won't generally reduce the number of SCF iterations significantly, but it CAN greatly increase them. This test is designed to ensure that the IncFock run doesn't require significantly more iterations (defined as more than 3 in this test); and this assert is the point at which the comparison is made.

Thank's for the clarification on the compare functions! With everything here, most likely what I would have in mind is assert compare(True, (niter_inc - niter_noinc) <= 3, "reduced by at least 3 iterations by incfock")

Ah, I see now, thanks. do you want an abs(niter_noinc - niter_inc) then?

You're welcome! Abs could be something we do, because it would catch cases where IncFock gives significantly lower SCF iterations than no-IncFock. It may be worth catching this in case it indicates some other error.

All right, I think this file has been largely refined based on the discussion here!

loriab

lgtm, thanks!

loriab · 2022-12-05T01:19:07Z

psi4/src/psi4/libfock/DFJCOSK.cc

-        build_K(D_eff, K_ao_);
-        timer_off("COSK");
+   	    build_K(D_ref_, K_ao_);
+   	    timer_off("COSK");


I wonder if your editor isn't counting spaces right or not converting tabs to 4 spaces. No harm or need to edit further, as we'll catch it in a clang-format someday.

That's a possibility. I use vim, and I'm pretty sure one can adjust vimrc to automatically convert tabs to 4 spaces. It's possible I haven't set this up yet.

psi4/src/psi4/libfock/jk.h

susilehtola · 2022-12-05T10:38:21Z

This reminds me, there is a technique to avoid recomputing every N iterations. Namely, I think Ahlrichs(?) described a procedure where one keeps track of the change of the density matrix from the reference one by setting D_0=0 and D_i = D_{i-1} + max{|Duv|} where Duv is the difference density matrix at iteration i and D_i tracks the changes. Instead of doing the reset every N iterations, one does a full rebuild when D_i >= eps because by then sufficient numerical error may have creeped into the calculation.

davpoolechem · 2022-12-05T14:24:45Z

This reminds me, there is a technique to avoid recomputing every N iterations. Namely, I think Ahlrichs(?) described a procedure where one keeps track of the change of the density matrix from the reference one by setting D_0=0 and D_i = D_{i-1} + max{|Duv|} where Duv is the difference density matrix at iteration i and D_i tracks the changes. Instead of doing the reset every N iterations, one does a full rebuild when D_i >= eps because by then sufficient numerical error may have creeped into the calculation.

Thank you for telling me about this! This is an intriguing idea overall. Essentially, instead of hard-setting a number of iterations to fully recompute the Fock matrix, one does it dynamically based on how much the density matrix has changed over time.

While I don't think this PR is the right place to implement such a scheme (with 1.7 is right around the corner; and since such a scheme would break standardization of IncFock between the different integral-direct JK builds Psi4 has currently, assuming we implement it only in DFJCOSK for this PR), it could be worth investigating in the future. The potential for removing a human-set parameter (INCFOCK_FULL_FOCK_EVERY in this case) is always appealing.

JonathonMisiewicz

Some nitpicky changes, but LGTM otherwise.

Three cheers for standardization!

psi4/driver/procrouting/scf_proc/scf_iterator.py

psi4/src/export_fock.cc

psi4/src/psi4/libfock/DFJCOSK.cc

JonathonMisiewicz · 2022-12-05T19:04:15Z

psi4/src/psi4/libfock/DFJCOSK.cc


        // If there is no previous pseudo-density, this iteration is normal
-        if(D_prev_.size() != njk) {
-            D_eff = D_ao_;
+        if(initial_iteration_ || D_prev_.size() != njk) {


This will need a CF pass later.

psi4/src/psi4/libfock/DFJCOSK.cc

Co-authored-by: Lori A. Burns <lori.burns@gmail.com>

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

davpoolechem · 2022-12-05T19:49:49Z

All right, I just did a final rebase. Given no issues with CI, and if others agree, this should be ready to merge!

loriab added this to the Psi4 1.7 milestone Dec 1, 2022

loriab added the scf Involves general SCF: convergence algorithms, RHF/UHF/ROHF/CUHF... label Dec 1, 2022

davpoolechem force-pushed the dpoole34/dfjcosk-incfock branch from a34da09 to 6d58793 Compare December 1, 2022 18:17

loriab reviewed Dec 2, 2022

View reviewed changes

davpoolechem force-pushed the dpoole34/dfjcosk-incfock branch from 6d58793 to 041f11f Compare December 4, 2022 19:05

loriab approved these changes Dec 5, 2022

View reviewed changes

JonathonMisiewicz approved these changes Dec 5, 2022

View reviewed changes

David Poole and others added 16 commits December 5, 2022 14:42

Use patch file to star DFJCOSK IncFock bells and whistles impl

cdbd55b

Add INCFOCK output to DFJCOSX during SCF iterations

62b937e

Add IncFock bells and whistles to DFJCOSK

e5185c2

Remove rest of references to COSX_INCFOCK in code

4e954f7

Add test for IncFock efficiency to test_dfjcosk.py pytest

211ed30

Update psi4/src/psi4/libfock/DFJCOSK.cc

d271175

Co-authored-by: Lori A. Burns <lori.burns@gmail.com>

Update psi4/src/psi4/libfock/DFJCOSK.cc

beb070e

Co-authored-by: Lori A. Burns <lori.burns@gmail.com>

Update psi4/src/psi4/libfock/DFJCOSK.cc

c452413

Co-authored-by: Lori A. Burns <lori.burns@gmail.com>

Update psi4/src/psi4/libfock/DFJCOSK.cc

c4cd264

Co-authored-by: Lori A. Burns <lori.burns@gmail.com>

Rework new assert in test_dfjcosk

60b3998

Further refinements to test_dfjcosk

b5f873d

Update description of initial_iteration_

5e52d57

Update psi4/src/psi4/libfock/DFJCOSK.cc

8acdb9f

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

Update psi4/driver/procrouting/scf_proc/scf_iterator.py

925c35b

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

Update psi4/src/export_fock.cc

b513bb2

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

Update psi4/src/psi4/libfock/DFJCOSK.cc

d6ed131

Co-authored-by: Jonathon Misiewicz <jonathon.paul.misiewicz@emory.edu>

davpoolechem force-pushed the dpoole34/dfjcosk-incfock branch from 6794e89 to d6ed131 Compare December 5, 2022 19:48

loriab merged commit 9842f7b into psi4:master Dec 5, 2022

davpoolechem mentioned this pull request Apr 11, 2023

Refactor Incremental Fock Builds in Chain-of-Spheres Exchange #2682

Closed

9 tasks

IncFock Standardization Part 3: DFJCOSK #2816

IncFock Standardization Part 3: DFJCOSK #2816

Uh oh!

Conversation

davpoolechem commented Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

User API & Changelog headlines

Dev notes & details

Questions

Checklist

Status

Uh oh!

davpoolechem commented Dec 1, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davpoolechem Dec 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loriab left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

susilehtola commented Dec 5, 2022

Uh oh!

davpoolechem commented Dec 5, 2022

Uh oh!

JonathonMisiewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davpoolechem commented Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

davpoolechem commented Dec 1, 2022 •

edited

Loading

davpoolechem Dec 4, 2022 •

edited

Loading

davpoolechem commented Dec 5, 2022 •

edited

Loading