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

Fixes bug in initialisation of ADC guess vectors #121

Merged
merged 2 commits into from
Mar 5, 2022

Conversation

obackhouse
Copy link
Contributor

@obackhouse obackhouse commented Mar 4, 2022

Description

When initialising the guess vectors for ADC, an identity matrix in the full space was briefly initialised, which is O(N^6) and O(N^8) in memory for IP/EA- and EE-ADC(2), respectively (!). This oversight is fine for these small examples but is very bad if people wanted to use the code as a reference for larger systems!

What are your new additions? Please provide a brief list.

  • Changes
    • Only initialises the required space for the guess vectors.
    • Correctly rotates final eigenvectors on convergence.

Status

  • Click when ready for review-and-merge

@obackhouse obackhouse mentioned this pull request Mar 4, 2022
Copy link
Member

@dgasmith dgasmith left a comment

Choose a reason for hiding this comment

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

Looks great!

@@ -57,6 +57,7 @@ def davidson(matrix, guesses, diag, maxiter=None, sort_via_abs=True, tol=1e-10,
de = np.linalg.norm(theta[:k] - theta_old)
if de < tol:
conv = True
b = np.dot(b, alpha)
Copy link
Member

Choose a reason for hiding this comment

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

Last iteration- got it.

@dgasmith dgasmith merged commit 81ad848 into psi4:master Mar 5, 2022
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.

2 participants