Begone, ERISieve! #2933

davpoolechem · 2023-04-28T15:29:57Z

Description

This PR is something of an independent project, although it is also a precursor of the continuation of #2695, as agreed upon by discussion there.

This PR completely eliminates the ERISieve class, and replaces all of its old functionalities with that of TwoBodyAOInt. ERISieve is a class in libmints that does many of the same things as TwoBodyAOInt. The amount of features that ERISieve has, that TwoBodyAOInt doesn't, is minimal. It's really just absolute redundancy. Currently, ERISieve only finds use within the PKJK class and its subalgorithms.

As mentioned, this PR eliminates ERISieve and replaces it with TwoBodyAOInt in all of the former's previous use cases. Any previously-utilized functionality that was present in ERISieve and not in TwoBodyAOInt (a couple functions determining basis function significance) were added to TwoBodyAOInt. Furthermore, any adjustments needed to support TwoBodyAOInt within the PKJK code were made.

With all of this, ERISieve was also ripped out of the code entirely. Maybe it could go into the attic.

User API & Changelog headlines

The core.ERISieve class has been removed, along with its associated functions core.ERISieve.build and core.ERISieve.shell_significant. The removed functionalities, specifically shell_significant, can be accessed via the construction of a TwoBodyAOInt object.

Dev notes & details

The ERISieve class in libmints has been removed from Psi4 and replaced with TwoBodyAOInt.
Functionalities in ERISieve that were not in TwoBodyAOInt and were required in the code, were added to TwoBodyAOInt.
The PKJK files have been adjusted as necessary to support the use of TwoBodyAOInt.

TODO

Remove ERISieve from all other external plugins (currently focusing on v2rdm_casscf).

Questions

I guess ERISieve can go into the attic now?

Checklist

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

Status

Ready for review
Ready for merge

TiborGY

Just a formality in this case, but marking arguments that are not modified in the function const is good practice.

psi4/src/psi4/libmints/twobody.h

davpoolechem · 2023-04-28T16:40:13Z

Ah, so it seems I made a mistake in my assumptions. The failed CI tells me this:

/home/runner/work/psi4/psi4/objdir/external/downstream/v2rdm_casscf/v2rdm_casscf_external-prefix/src/v2rdm_casscf_external/threeindexintegrals.cc:33:10: fatal error: psi4/libmints/sieve.h: No such file or directory #include <psi4/libmints/sieve.h>

It seems that PKJK is, in fact, not the only place that uses ERISieve. This v2rdm_casscf plugin does, as well.

loriab · 2023-04-28T17:09:31Z

v2rdm strikes again! :-)

fwiw, the original author (DePrince) isn't dedicated to preserving the plugin, but it's handy for catching things that may affect downstream users. Right now we're pulling from https://github.com/loriab/v2rdm_casscf/tree/v2rdm8, so if you want to PR a switchout of ERISeive to there for post-1.8, that'd be fine. In any case, it might be worth getting in a deprecation warning now for v1.8 on ERISeive.

davpoolechem · 2023-04-28T17:44:59Z

v2rdm strikes again! :-)

fwiw, the original author (DePrince) isn't dedicated to preserving the plugin, but it's handy for catching things that may affect downstream users. Right now we're pulling from https://github.com/loriab/v2rdm_casscf/tree/v2rdm8, so if you want to PR a switchout of ERISeive to there for post-1.8, that'd be fine. In any case, it might be worth getting in a deprecation warning now for v1.8 on ERISeive.

Thank you for the heads-up on this! I'll work on getting rid of ERISieve there, as well, and deprecate ERISieve here for 1.8. With this PR being connected to v2rdm_casscf, it'd probably be wise to make this PR post-1.8.

davpoolechem · 2023-04-28T18:22:36Z

I just put up #2935 to deprecate ERISieve. That one should go in before this one.

Co-authored-by: TiborGY <tibor.gyori@chem.u-szeged.hu>

davpoolechem · 2023-05-24T19:21:34Z

I officially opened up a PR for v2rdm_casscf, at loriab/v2rdm_casscf#5, which removes ERISieve from v2rdm_casscf and replaces it with TwoBodyAOInt. Once that is added in, we can progress with this PR, as that is the current bottleneck preventing this PR from passing CI testing. Ideally, there's no fun surprises after that.

davpoolechem · 2023-05-25T18:56:25Z

All right, the necessary v2rdm_casscf changes have been made and incorporated into this PR. So, the review process can proceed!

loriab · 2023-05-25T20:01:34Z

psi4/driver/p4util/python_helpers.py

-        category=FutureWarning,
-        stacklevel=2)
-
-    return core.ERISieve(orbital_basis, cutoff, do_csam)


you could raise an UpgradeHelper build-48s/src/lib/libint/library-prefix/src/library-build/src/configuration.cc saying how to replace this instead of deleting completely. that is, instead of warning and continuing, it stops with advice. up to you.

helps if I send a link to the code rather than a path on my local machine :-)

psi4/psi4/driver/p4util/python_helpers.py

Line 824 in 819c588

raise UpgradeHelper(key.upper(), key.upper()[:-2], 1.6, " Note the Debye -> a.u. units change.")

I think this is a pretty good idea! I will give it a look.

So I do have a version of the code that raises an UpgradeHelper for calling core.ERISieve.build instead of deleting ERISieve completely, at https://github.com/davpoolechem/psi4/tree/dpoole34/erisieve-upgrade-helper. That is probably better from an API maintenance approach than simply deleting ERISieve, but we would probably want to close down this PR and push the other branch instead if we wanted the UpgradeHelper approach. Which is totally fine by me, to be honest!

ok, new PR is fine. or you could push your alternate branch to this PR's remote branch and continue here.

I'm thinking a new PR would be easier, so I will likely do that.

davpoolechem · 2023-05-31T15:23:09Z

Closing this one up in favor of #2974.

TiborGY reviewed Apr 28, 2023

View reviewed changes

psi4/src/psi4/libmints/twobody.h Outdated Show resolved Hide resolved

psi4/src/psi4/libmints/twobody.h Outdated Show resolved Hide resolved

davpoolechem mentioned this pull request Apr 28, 2023

Replace ERISieve with TwoBodyAOInt loriab/v2rdm_casscf#5

Merged

loriab added this to the Psi4 1.9 milestone Apr 28, 2023

davpoolechem mentioned this pull request Apr 28, 2023

Deprecate ERISieve #2935

Merged

11 tasks

davpoolechem force-pushed the dpoole34/erisieve branch from 1c23e23 to a15ecf7 Compare May 4, 2023 15:06

David Poole and others added 8 commits May 24, 2023 15:04

First effort at replacing ERISieve with TwoBodyAOInt

99b1fdb

Comment out debug output again

7ddd749

More fixing-up of PK_workers debug comments

544d588

Whitespace cleanup

2441d6e

Say goodbye to ERISieve!

9e6ca5a

Update psi4/src/psi4/libmints/twobody.h

09af5b1

Co-authored-by: TiborGY <tibor.gyori@chem.u-szeged.hu>

Update psi4/src/psi4/libmints/twobody.h

968a7c8

Co-authored-by: TiborGY <tibor.gyori@chem.u-szeged.hu>

Remove ERISieve Python helper functions

6097690

davpoolechem force-pushed the dpoole34/erisieve branch from a15ecf7 to 6097690 Compare May 24, 2023 19:17

Update v2rdm_casscf commit hash with new changes

819c588

loriab reviewed May 25, 2023

View reviewed changes

davpoolechem closed this May 31, 2023

davpoolechem mentioned this pull request May 31, 2023

Begone, ERISieve (Kind of)! #2974

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Begone, ERISieve! #2933

Begone, ERISieve! #2933

Uh oh!

davpoolechem commented Apr 28, 2023 •

edited

Loading

Uh oh!

TiborGY left a comment

Uh oh!

Uh oh!

Uh oh!

davpoolechem commented Apr 28, 2023 •

edited

Loading

Uh oh!

loriab commented Apr 28, 2023

Uh oh!

davpoolechem commented Apr 28, 2023 •

edited

Loading

Uh oh!

davpoolechem commented Apr 28, 2023

Uh oh!

davpoolechem commented May 24, 2023

Uh oh!

davpoolechem commented May 25, 2023 •

edited

Loading

Uh oh!

loriab May 25, 2023

Uh oh!

loriab May 25, 2023

Uh oh!

davpoolechem May 26, 2023

Uh oh!

davpoolechem May 31, 2023 •

edited

Loading

Uh oh!

loriab May 31, 2023

Uh oh!

davpoolechem May 31, 2023

Uh oh!

davpoolechem commented May 31, 2023

Uh oh!

Uh oh!

Begone, ERISieve! #2933

Begone, ERISieve! #2933

Uh oh!

Conversation

davpoolechem commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

User API & Changelog headlines

Dev notes & details

TODO

Questions

Checklist

Status

Uh oh!

TiborGY left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

davpoolechem commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loriab commented Apr 28, 2023

Uh oh!

davpoolechem commented Apr 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davpoolechem commented Apr 28, 2023

Uh oh!

davpoolechem commented May 24, 2023

Uh oh!

davpoolechem commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loriab May 25, 2023

Choose a reason for hiding this comment

Uh oh!

loriab May 25, 2023

Choose a reason for hiding this comment

Uh oh!

davpoolechem May 26, 2023

Choose a reason for hiding this comment

Uh oh!

davpoolechem May 31, 2023 • 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!

loriab May 31, 2023

Choose a reason for hiding this comment

Uh oh!

davpoolechem May 31, 2023

Choose a reason for hiding this comment

Uh oh!

davpoolechem commented May 31, 2023

Uh oh!

Uh oh!

davpoolechem commented Apr 28, 2023 •

edited

Loading

davpoolechem commented Apr 28, 2023 •

edited

Loading

davpoolechem commented Apr 28, 2023 •

edited

Loading

davpoolechem commented May 25, 2023 •

edited

Loading

davpoolechem May 31, 2023 •

edited

Loading