Skip to content

SAPT0 CPHF Uniform Treatment #3105

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
Merged
loriab merged 7 commits into from
Dec 19, 2023
Merged

SAPT0 CPHF Uniform Treatment #3105

loriab merged 7 commits into from
Dec 19, 2023

Conversation

@Awallace3
Copy link
Contributor

@Awallace3 Awallace3 commented Dec 13, 2023

Description

This changes the name of the controller for all three SAPT modules and standardizes their convergences as residual only with default of 1e-8.

User API & Changelog headlines

  • RN 1
  • RN 2

Dev notes & details

  • Feature1
  • Feature2

Questions

  • Question1

Checklist

Status

  • Ready for review
  • Ready for merge

@konpat
Copy link
Contributor

konpat commented Dec 17, 2023

Thank you for doing this, Austin! This is very helpful.

Do I understand correctly that Ed's SAPT0 now no longer uses E_CONVERGENCE as a part of the criterion? Then, I think these lines have to go to remove the unused option from the docs.

@loriab
Copy link
Member

loriab commented Dec 17, 2023

Right, Ed's SAPT's CPHF still prints E but no longer considers deltaE for convergence. (There's a few more notes on the Google Doc.) Austin's away -- I'll get this compiling.

Was there another message (appears in my email) that got deleted on GH?

@konpat
Copy link
Contributor

konpat commented Dec 17, 2023

Was there another message (appears in my email) that got deleted on GH?

Yes, I deleted it when I realized my criticism was totally false. I apologize for the confusion.

@loriab
Copy link
Member

loriab commented Dec 17, 2023 •
edited
Loading

Was there another message (appears in my email) that got deleted on GH?

Yes, I deleted it when I realized my criticism was totally false. I apologize for the confusion.

Ok, no problem, thanks. I just keep losing things in email, and I remembered the suggetion sounded likely. :-)

@konpat
Copy link
Contributor

konpat commented Dec 18, 2023

LGTM now.

@loriab loriab added this to the Psi4 1.10 milestone Dec 18, 2023
@loriab loriab added the sapt For issues about SAPT and its many flavors. label Dec 18, 2023
@loriab loriab enabled auto-merge December 18, 2023 21:28
@loriab loriab added this pull request to the merge queue Dec 18, 2023
Merged via the queue into psi4:master with commit 79ac936 Dec 19, 2023
TiborGY pushed a commit to TiborGY/psi4 that referenced this pull request Dec 26, 2023
@Awallace3 @loriab @TiborGY
SAPT0 CPHF Uniform Treatment (psi4#3105)
526b380 
* compile_commands

* fixed SAPT0 CPHF convergence criteria keyword defaults

* removed cphf_e_convergence, restored sapt-compare test, and returned CMakeLists.txt to normal

* COPHF_R_CONVERGENCE

* more libsapt e/d_conv

* fix docs

* some warnings

---------

Co-authored-by: Lori A. Burns <[email protected]>
@loriab loriab mentioned this pull request Jan 8, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loriab loriab loriab approved these changes

@davpoolechem davpoolechem davpoolechem approved these changes

+1 more reviewer

@konpat konpat konpat approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

sapt For issues about SAPT and its many flavors.

Projects

None yet

Milestone

Psi4 1.10

Development

Successfully merging this pull request may close these issues.

4 participants

@Awallace3 @konpat @loriab @davpoolechem