Skip to content

Conversation

JonathonMisiewicz
Copy link
Contributor

@JonathonMisiewicz JonathonMisiewicz commented Mar 26, 2022

Description

This PR makes changes so that Psi can do DIIS on an ambit.BlockedTensor, as required by the forte plugin. This PR will not work until this ambit PR is merged, but passes locally. The test suite tells us little about this one, sadly.

@loriab, I leave it up to you whether this warrants an update to the version of ambit that Psi takes.

@fevangelista @lcyyork

Status

  • Ready for review
  • Leave merging to me

Copy link
Member

@loriab loriab left a comment

Choose a reason for hiding this comment

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

I don't think psi fails in the presence of the longstanding ambit with this PR, so no need to insist on new tag. You could update https://github.com/psi4/psi4/blob/master/external/upstream/ambit/CMakeLists.txt#L30 here once the ambit PR is merged (do a few characters of sha instead of the tag).

lgtm!

@zachglick
Copy link
Contributor

This is a nice feature! Is there a good place (perhaps a docstring) to explicitly state that DIIS is now compatible with ambit tensors?

@JonathonMisiewicz
Copy link
Contributor Author

This is a nice feature! Is there a good place (perhaps a docstring) to explicitly state that DIIS is now compatible with ambit tensors?

Added.

Some changes were requested in the ambit PR, so I'll need to double-check that the forte tests still work before I can merge this in.

@JonathonMisiewicz JonathonMisiewicz merged commit 051a5da into psi4:master Mar 26, 2022
@JonathonMisiewicz JonathonMisiewicz deleted the hattrick branch March 26, 2022 22:17
zachglick pushed a commit to zachglick/psi4 that referenced this pull request Apr 18, 2022
* Fixes for Forte: can now DIIS ambit.BlockedTensor

* Misc. changes
zachglick pushed a commit to zachglick/psi4 that referenced this pull request Apr 18, 2022
* Fixes for Forte: can now DIIS ambit.BlockedTensor

* Misc. changes
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.

5 participants