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

Solver convergence #175

Merged
merged 17 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion adcc/backends/molsturm.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def convert_scf_to_dict(scfres):
data["occupation_f"][n_oa:n_oa + n_beta] = 1.

data["energy_scf"] = scfres["energy_ground_state"]
data["conv_tol"] = 10 * scfres["final_error_norm"]
data["conv_tol"] = scfres["final_error_norm"]
data["orbcoeff_fb"] = scfres["orbcoeff_bf"].transpose().copy()

# Compute electric and nuclear multipole moments
Expand Down
2 changes: 1 addition & 1 deletion adcc/backends/psi4.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ def get_conv_tol(self):
conv_tol = psi4.core.get_option("SCF", "E_CONVERGENCE")
# RMS value of the orbital gradient
conv_tol_grad = psi4.core.get_option("SCF", "D_CONVERGENCE")
threshold = max(10 * conv_tol, conv_tol_grad)
threshold = max(conv_tol, conv_tol_grad)
return threshold

def get_restricted(self):
Expand Down
5 changes: 2 additions & 3 deletions adcc/backends/pyscf.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,9 @@ def get_backend(self):

def get_conv_tol(self):
if self.scfres.conv_tol_grad is None:
conv_tol_grad = np.sqrt(self.scfres.conv_tol)
conv_tol = self.scfres.conv_tol
else:
conv_tol_grad = self.scfres.conv_tol_grad
conv_tol = max(10 * self.scfres.conv_tol, conv_tol_grad)
conv_tol = max(self.scfres.conv_tol, self.scfres.conv_tol_grad**2)
return conv_tol

def get_restricted(self):
Expand Down
10 changes: 5 additions & 5 deletions adcc/backends/test_backends_pcm.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ def template_pcm_ptlr_formaldehyde(self, basis, method, backend):

assert_allclose(scfres.energy_scf, result["energy_scf"], atol=1e-8)

state = adcc.run_adc(scfres, method=method, n_singlets=5,
conv_tol=1e-7, environment="ptlr")
state = adcc.run_adc(scfres, method=method, n_singlets=5, conv_tol=1e-7,
max_subspace=24, environment="ptlr")

# compare ptLR result to LR data
assert_allclose(state.excitation_energy,
Expand Down Expand Up @@ -78,7 +78,7 @@ def template_pcm_linear_response_formaldehyde(self, basis, method, backend):
assert len(matrix.extra_terms)

state = adcc.run_adc(matrix, n_singlets=5, conv_tol=1e-7,
environment=False)
max_subspace=24, environment=False)
assert_allclose(
state.excitation_energy_uncorrected,
result["lr_excitation_energy"],
Expand All @@ -101,8 +101,8 @@ def template_pcm_linear_response_formaldehyde(self, basis, method, backend):
adcc.run_adc(scfres, method=method, n_singlets=5)

# automatically add coupling term
state = adcc.run_adc(scfres, method=method, n_singlets=5,
conv_tol=1e-7, environment="linear_response")
state = adcc.run_adc(scfres, method=method, n_singlets=5, conv_tol=1e-7,
max_subspace=24, environment="linear_response")
assert_allclose(
state.excitation_energy_uncorrected,
result["lr_excitation_energy"],
Expand Down
13 changes: 3 additions & 10 deletions adcc/solver/davidson.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,8 @@ def form_residual(rval, rvec):
epair_mask = select_eigenpairs(rvals, n_ep, which)
state.eigenvalues = rvals[epair_mask]
state.residuals = [residuals[i] for i in epair_mask]
state.residual_norms = np.array([r @ r for r in state.residuals])
# TODO This is misleading ... actually residual_norms contains
# the norms squared. That's also the used e.g. in adcman to
# check for convergence, so using the norm squared is fine,
# in theory ... it should just be consistent. I think it is
# better to go for the actual norm (no squared) inside the code
#
# If this adapted, also change the conv_tol to tol conversion
# inside the Lanczos procedure.
state.residual_norms = np.array([np.sqrt(r @ r)
for r in state.residuals])

callback(state, "next_iter")
state.timer.restart("iteration")
Expand Down Expand Up @@ -330,7 +323,7 @@ def eigsh(matrix, guesses, n_ep=None, max_subspace=None,
max_subspace : int or NoneType, optional
Maximal subspace size
conv_tol : float, optional
Convergence tolerance on the l2 norm squared of residuals to consider
Convergence tolerance on the l2 norm of residuals to consider
them converged
which : str, optional
Which eigenvectors to converge to (e.g. LM, LA, SM, SA)
Expand Down
24 changes: 3 additions & 21 deletions adcc/solver/lanczos.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ def form_residual(rval, rvec):
if i in epair_mask]
rnorms = np.array([np.sqrt(r @ r) for r in residuals])

# Note here the actual residual norm (and not the residual norm squared)
# is returned.
return eigenvectors, residuals, rnorms


Expand All @@ -103,10 +101,6 @@ def amend_true_residuals(state, subspace, rvals, rvecs, epair_mask):
res = compute_true_residuals(subspace, rvals, rvecs, epair_mask)
state.eigenvectors, state.residuals, state.residual_norms = res

# TODO For consistency with the Davidson the residual norms are
# squared to give output in the same order of magnitude.
state.residual_norms = state.residual_norms**2

return state


Expand Down Expand Up @@ -152,7 +146,7 @@ def lanczos_iterations(iterator, n_ep, min_subspace, max_subspace, conv_tol=1e-9
max_subspace : int
Maximal subspace size
conv_tol : float, optional
Convergence tolerance on the l2 norm squared of residuals to consider
Convergence tolerance on the l2 norm of residuals to consider
them converged
which : str, optional
Which eigenvectors to converge to (e.g. LM, LA, SM, SA)
Expand All @@ -168,14 +162,6 @@ def lanczos_iterations(iterator, n_ep, min_subspace, max_subspace, conv_tol=1e-9
def callback(state, identifier):
pass

# TODO For consistency with the Davidson the conv_tol is interpreted
# as the residual norm *squared*. Arnoldi, however, uses the actual norm
# to check for convergence and so on. See also the comment in Davidson
# around the line computing state.residual_norms
#
# See also the squaring of the residual norms below
tol = np.sqrt(conv_tol)

if state is None:
state = LanczosState(iterator)
callback(state, "start")
Expand All @@ -191,12 +177,12 @@ def callback(state, identifier):

if debug_checks:
eps = np.finfo(float).eps
orthotol = max(tol / 1000, subspace.n_problem * eps)
orthotol = max(conv_tol / 1000, subspace.n_problem * eps)
orth = subspace.check_orthogonality(orthotol)
state.subspace_orthogonality = orth

is_rval_converged, eigenpair_error = check_convergence(subspace, rvals,
rvecs, tol)
rvecs, conv_tol)

# Update state
state.n_iter += 1
Expand All @@ -211,10 +197,6 @@ def callback(state, identifier):
state.residual_norms = eigenpair_error[epair_mask]
converged = np.all(is_rval_converged[epair_mask])

# TODO For consistency with the Davidson the residual norms are squared
# again to give output in the same order of magnitude.
state.residual_norms = state.residual_norms**2

callback(state, "next_iter")
state.timer.restart("iteration")

Expand Down
16 changes: 9 additions & 7 deletions adcc/test_functionality.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,14 @@
class TestFunctionalityBase(unittest.TestCase):
def base_test(self, system, method, kind, generator="atd", prefix="",
test_mp=True, **args):
if method == "adc1" and "cn_sto3g" in system:
pytest.xfail("For CN/STO-3G adc1, the Davidson sometimes converges to "
"zero for unknown reasons.")
if prefix:
prefix += "-"
hf = cache.hfdata[system]
if generator == "atd":
refdata = cache.reference_data[system]
args["conv_tol"] = 3e-9
else:
refdata = cache.adcc_reference_data[system]
ref = refdata[prefix.replace("_", "-") + method][kind]
Expand Down Expand Up @@ -117,15 +119,15 @@ def base_test(self, system, method, kind, generator="atd", prefix="",
# Test we do not use too many iterations
if smallsystem:
n_iter_bound = {
"adc0": 1, "adc1": 4, "adc2": 10, "adc2x": 14, "adc3": 13,
"cvs-adc0": 1, "cvs-adc1": 4, "cvs-adc2": 5, "cvs-adc2x": 12,
"cvs-adc3": 13,
"adc0": 1, "adc1": 6, "adc2": 20, "adc2x": 18, "adc3": 15,
"cvs-adc0": 1, "cvs-adc1": 4, "cvs-adc2": 5, "cvs-adc2x": 18,
"cvs-adc3": 17,
}[method]
else:
n_iter_bound = {
"adc0": 1, "adc1": 8, "adc2": 16, "adc2x": 17, "adc3": 17,
"cvs-adc0": 1, "cvs-adc1": 7, "cvs-adc2": 16, "cvs-adc2x": 18,
"cvs-adc3": 17,
"adc0": 1, "adc1": 10, "adc2": 20, "adc2x": 25, "adc3": 25,
"cvs-adc0": 1, "cvs-adc1": 9, "cvs-adc2": 27, "cvs-adc2x": 31,
"cvs-adc3": 19,
}[method]
assert res.n_iter <= n_iter_bound

Expand Down
4 changes: 2 additions & 2 deletions adcc/testdata/0_download_testdata.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash

SOURCE="https://q-chem.de/adcc_testdata/0.5.0/"
SOURCE="https://wwwagdreuw.iwr.uni-heidelberg.de/adcc_test_data/0.6.0/"
DATAFILES=(
ch2nh2_sto3g_hfdata.hdf5
ch2nh2_sto3g_hfimport.hdf5
Expand Down Expand Up @@ -180,7 +180,7 @@ echo "Updating testdata ... please wait."

download() {
if which wget &> /dev/null; then
wget -w 1 -qN --show-progress $@
wget -w 1 -qN --show-progress --no-check-certificate $@
else
echo "wget not installed" >&2
exit 1
Expand Down
Loading
Loading