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.

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

Begone, ERISieve (Actually!) #3009

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

davpoolechem
Copy link
Contributor

@davpoolechem davpoolechem commented Jul 11, 2023

Description

The final removal of the ERISieve class, and the culmination of the work in PRs #2935 and #2974. This fully, and completely, gets rid of ERISieve from Psi4, with TwoBodyAOInt now being the class of choice for ERI computations.

Note: Now that v1.9 has released officially with ERISieve throwing an UpgradeHelper exception, I am opening this up for review.

User API & Changelog headlines

  • The ERISieve class has been completely removed from Psi4. For previous ERISieve functionalities, the TwoBodyAOInt class should be used instead.

Dev notes & details

  • Gets rid of all occurrences of the ERISieve class in Psi4, from build files to UpgradeHelper Python functions to C++ implementation details.
  • Slight update of integrals documentation with a better description of what is occurring.

Questions

N/A

Checklist

Status

  • Ready for review
  • Ready for merge

@davpoolechem davpoolechem force-pushed the dpoole34/erisieve-removal branch 2 times, most recently from dde50c5 to d9f144a Compare August 28, 2023 13:17
@davpoolechem davpoolechem force-pushed the dpoole34/erisieve-removal branch 2 times, most recently from bb24266 to d766ca3 Compare October 2, 2023 13:04
@davpoolechem davpoolechem marked this pull request as ready for review December 8, 2023 14:19
*
*/

class PSI_API __attribute__((deprecated(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this PR removes a class marked as PSI_API, I think it should be mentioned in the API changes section of the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Done and done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants