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

use findif for ccsd(t) conv gradients most of time #2943

Merged
merged 5 commits into from May 8, 2023

Conversation

loriab
Copy link
Member

@loriab loriab commented May 1, 2023

Description

@lothian, the primary files to look at are procedures/proc.py, cc.rst, preview_capabilities_ccenergy.rst, and (for example) cc13b/input.dat

User API & Changelog headlines

  • Conventional ccsd(t) gradients will now proceed as finite-difference calculations, which are much more memory efficient. If you still want to access the analytic gradients, add set qc_module ccenergy.

Dev notes & details

  • the main change is disabling default conv rhf/uhf ccsd(t) analytic gradients through ccenergy in proc.py . they're still accessible by setting qc_module=ccenergy explicitly
  • changes to tests so that we're still testing the gradients
  • changes to the capabilities auto-documentation that starts with stdsuite, gets stored in samples/stdsuite_psi4.txt, and gets processed into docs tables, a viz preview of which are stored in sphinxman/source/.
  • simpler docs-building environment!

Checklist

closes #2913

Status

  • Ready for review
  • Ready for merge

@loriab loriab added this to the Psi4 1.8 milestone May 1, 2023
@loriab loriab marked this pull request as draft May 1, 2023 03:25
@loriab loriab added documentation cc For all issues involving the CC module, ground-state energies to response properties. labels May 3, 2023
@loriab loriab requested a review from lothian May 3, 2023 08:19
@loriab loriab marked this pull request as ready for review May 3, 2023 08:19
Copy link
Contributor

@lothian lothian left a comment

Choose a reason for hiding this comment

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

I wonder if it's the syntax qc_module="ccenergy" is confusing since that's the name of the CC energy code, but it's signaling to compute gradients analytically. I'm not sure what I'd prefer, but I don't think this will be obvious to most users.

@loriab
Copy link
Member Author

loriab commented May 3, 2023

I wonder if it's the syntax qc_module="ccenergy" is confusing since that's the name of the CC energy code, but it's signaling to compute gradients analytically. I'm not sure what I'd prefer, but I don't think this will be obvious to most users.

I agree there's not a great name for the cc suite. Only alternative I've seen is cc*. cc alone seems too generic.

It looks like the qc_module=ccenergy came about for this role between beta5 and v1.0: https://github.com/psi4/psi4archive/blob/1.0.x/src/bin/psi4/read_options.cc#L165-L168 . So it's longstanding, though probably little used. If you think of a preferred alias, it'd be easy enough to re-route, I suspect.

@lothian
Copy link
Contributor

lothian commented May 3, 2023

I wonder if it's the syntax qc_module="ccenergy" is confusing since that's the name of the CC energy code, but it's signaling to compute gradients analytically. I'm not sure what I'd prefer, but I don't think this will be obvious to most users.

I agree there's not a great name for the cc suite. Only alternative I've seen is cc*. cc alone seems too generic.

It looks like the qc_module=ccenergy came about for this role between beta5 and v1.0: https://github.com/psi4/psi4archive/blob/1.0.x/src/bin/psi4/read_options.cc#L165-L168 . So it's longstanding, though probably little used. If you think of a preferred alias, it'd be easy enough to re-route, I suspect.

OK, then let's go ahead.

@lothian lothian self-requested a review May 3, 2023 14:48
@@ -242,6 +242,7 @@ def extract_modules(winnowed):
("cisd", None): (", ci\ *n*", "Arbitrary-order *n* through DETCI is inefficient byproduct of CI", ["fnocc"]),
("zapt2", None): (", zapt\ *n*", "Arbitrary-order *n* through DETCI is inefficient byproduct of CI", None),
("mp4", None): (", mp\ *n*", "Arbitrary-order *n* through DETCI is inefficient byproduct of CI", ["fnocc"]),
("ccsd(t)", "CCENERGY"): ("FN", "Analytic gradients for conventional all-electron RHF/UHF computations can be requested through |globals__qc_module|\ ``=ccenergy``, but their scaling is best suited to small molecules.", None),
Copy link
Contributor

Choose a reason for hiding this comment

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

When I see the word "scaling" in this context I think of CPU-time, not RAM requirement. If the main problem with the current implementation is the amount of RAM required, I think something along the lines of "but the amount of RAM required scales steeply with system size." would get that point across better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Memory is what the output files show running out. But if I recall old conversations correctly, it's N^9 scaling, not the expected N^8.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's N^8 or N^9. It should be N^7, but the prefactor is terrible in my implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

eek, yes, N^8 instead of N^7 is what I meant to write. Good to know that the issue is actually prefactor.

If anyone wants to clarify the message, the document_capabilities.py file is the single source. Any edits can get propagated to the tables in a future docs build.

@loriab loriab added this pull request to the merge queue May 8, 2023
Merged via the queue into psi4:master with commit 5b0daac May 8, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cc For all issues involving the CC module, ground-state energies to response properties. documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Out of memory running CCSD(T) on HCl dimer
5 participants