-
Notifications
You must be signed in to change notification settings - Fork 440
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.
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
conventional REMP2 and OREMP2 #2653
Conversation
Excellent, thanks for your tremendous effort.
MP2 should also be available from the guess. But it is of course better to not print/store something than to print something wrong. I might be mistaken, but given that |
This is completely correct. The lone OLCCD algorithm in |
Great, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
t2_amps_remp(); // <- the only actual modification compared to regular CEPA(0)/D | ||
timer_off("T2"); | ||
timer_on("CEPA Energy"); | ||
cepa_energy(); | ||
cepa_diis(t2_diis); // <- CEPA diis can be reused without modifications | ||
cepa_chemist(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good rationale for renaming functions, so that things labeled cepa
are actually cepa
only. I'd be thrilled if we could condense this and the CEPA code into a single function that just passes a flag around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the future ...
@behnle, were there any changes from the review that you wanted to PR into this PR? Otherwise, I'll force push my accumulated responses. |
As far as i can see, this is fine as is. |
Jonathon's comments all addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍺
@@ -107,6 +109,8 @@ def runner_asserter(inp, subject, method, basis, tnm): | |||
# _recorder(qcprog, qc_module_in, driver, method, reference, fcae, scf_type, corl_type, "error", "nyi: " + reason) | |||
return | |||
|
|||
psi4.set_output_file("asdf") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"asdf" intentional or placeholder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it's a hack solution to a mystery. Each job is supposed to produce a .out
and a .log
file. The log file shows up, but the out file doesn't even though I cancel deletions in conftest.py. If I change the base to asdf
, both files show up, so I can debug. So, intentional but temporary.
global_dpd_->buf4_init(&Taa, PSIF_OCC_DPD, 0, ID("[O,O]"), ID("[V,V]"), ID("[O,O]"), ID("[V,V]"), 0, | ||
"T2 <OO|VV>"); | ||
global_dpd_->buf4_init(&Tbb, PSIF_OCC_DPD, 0, ID("[o,o]"), ID("[v,v]"), ID("[o,o]"), ID("[v,v]"), 0, | ||
"T2 <oo|vv>"); | ||
global_dpd_->buf4_init(&Tab, PSIF_OCC_DPD, 0, ID("[O,o]"), ID("[V,v]"), ID("[O,o]"), ID("[V,v]"), 0, | ||
"T2 <Oo|Vv>"); | ||
t2_diis = DIISManager(maxdiis_, "CEPA DIIS T2 Amps", DIISManager::RemovalPolicy::LargestError, DIISManager::StoragePolicy::InCore); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for the different the StoragePolicy
's for RHF and UHF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No... Should probably be both OnDisk
. I also propose that the initialization of t2_diis is taken out of the
if (reference_ == "RESTRICTED") {
...
}
else if (reference_ == "UNRESTRICTED") {
...
}
block. It seems redundant to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the different policies is what Ugur had. But makes sense to unify them. I'll do it in PR 3, though, which will go up today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context, during my previous changes in the occ
module, I unified them as behnle
suggested. This was an unintended behavior change. Lori is changing back to how Ugur had it before my interventions.
Three cheers for three approvals. I'm deferring merge to Lori on a PR like this. |
Thank you all very mch guys, if i ever meet you, i definitely owe you a 🍺 . |
You're very welcome. Thanks for (1) the new methods, (2) the additional motivation to address the technical debt in occ/dfocc, and (3) your fixes to dfocc convergence to make (2) possible :-) . I owe you a return 🍺 and certainly a Psi mug. We'll have to coordinate addresses/conferences on the latter. |
Description
REMP methods added to
occ
and attendant testing and routing.This is PR 1/? in the mega-dfocc-remp series.
PR Background (not of general interest)
There was https://github.com/loriab/psi4/tree/dfocc2 by @bozkaya and @yavuzalagoz that added to
dfocc
(1) non-oo uhf ccsd-level E & G, (2) oo ccd-level E & G, and (3) fno everything. Then, there was #2354 that added remp2 E & G toocc
anddfocc
and made extensive improvements todfocc
in the matter of combined DIIS (vital for convergence) and int overflows. All based on pre-DIIS overhaul #2369 . To untangle this, I started from master, cherry-picked the dfocc2 commits, cherry-picked the #2354 commits, modernized the DIIS calls, then added the neglected stdsuite testing and attendant qcvar fixups for existing occ/dfocc methods and new methods remp2 and categories (1) and (2) from dfocc2 branch. That mega-changeset lives at #2633 for now. This PR breaks off theocc
portion for review.Todos
occ
module, as originally proposed in [Feature addition] Add the REMP and OO-REMP hybrid perturbation theories to OCC/DFOCC #2354Wfn.energy_
, stop setting ROHF MP3 and MP2.5 out of caution.Questions
occ
code, but do look it over if you want.Checklist
Status