From 86e62e0cfd4a0d411f308a0adcb6c52fe64b0431 Mon Sep 17 00:00:00 2001 From: Maximilian Scheurer Date: Wed, 19 May 2021 16:35:49 +0200 Subject: [PATCH] reviews --- adcc/AdcMatrix.py | 15 ++++++++------- adcc/ExcitedStates.py | 8 ++++---- adcc/backends/psi4.py | 7 +++---- adcc/backends/pyscf.py | 10 +++------- adcc/backends/veloxchem.py | 7 +++---- adcc/test_AdcMatrix.py | 14 +++++++------- adcc/workflow.py | 26 ++++++++++++++------------ docs/calculations.rst | 31 +++++++++++++++++++++++++++++++ libadcc/TensorImpl.hh | 2 +- 9 files changed, 74 insertions(+), 46 deletions(-) diff --git a/adcc/AdcMatrix.py b/adcc/AdcMatrix.py index e381b316..2028252e 100644 --- a/adcc/AdcMatrix.py +++ b/adcc/AdcMatrix.py @@ -112,6 +112,14 @@ def __init__(self, method, hf_or_mp, block_orders=None, intermediates=None, if not isinstance(method, AdcMethod): method = AdcMethod(method) + if diagonal_precomputed: + if not isinstance(diagonal_precomputed, AmplitudeVector): + raise TypeError("diagonal_precomputed needs to be" + " an AmplitudeVector.") + if diagonal_precomputed.needs_evaluation: + raise ValueError("diagonal_precomputed must already" + " be evaluated.") + self.timer = Timer() self.method = method self.ground_state = hf_or_mp @@ -159,12 +167,6 @@ def __init__(self, method, hf_or_mp, block_orders=None, intermediates=None, # TODO Rename to self.block in 0.16.0 self.blocks_ph = {bl: blocks[bl].apply for bl in blocks} if diagonal_precomputed: - if not isinstance(diagonal_precomputed, AmplitudeVector): - raise TypeError("diagonal_precomputed needs to be" - " an AmplitudeVector.") - if diagonal_precomputed.needs_evaluation: - raise ValueError("diagonal_precomputed must already" - " be evaluated.") self.__diagonal = diagonal_precomputed else: self.__diagonal = sum(bl.diagonal for bl in blocks.values() @@ -194,7 +196,6 @@ def patched_apply(ampl, original=orig_app, other=other_app): self.blocks_ph[sp] = patched_apply other_diagonal = sum(bl.diagonal for bl in other.blocks.values() if bl.diagonal) - # iadd does not work with numbers self.__diagonal = self.__diagonal + other_diagonal self.__diagonal.evaluate() self.extra_terms.append(other) diff --git a/adcc/ExcitedStates.py b/adcc/ExcitedStates.py index 3ce9e641..0e5e45d5 100644 --- a/adcc/ExcitedStates.py +++ b/adcc/ExcitedStates.py @@ -208,12 +208,12 @@ def __iadd__(self, other): for k in other: self += k else: - raise TypeError("Can only add EnergyCorrection (or list" - " of EnergyCorrection) to" - f" ExcitedState, not '{type(other)}'") + return NotImplemented return self def __add__(self, other): + if not isinstance(other, (EnergyCorrection, list)): + return NotImplemented ret = ExcitedStates(self, self.method, self.property_method) ret += other return ret @@ -376,7 +376,7 @@ def describe(self, oscillator_strengths=True, rotatory_strengths=False, ev=self.excitation_energy[i] * eV, **fields) text += separator + "\n" if len(self._excitation_energy_corrections): - head_corr = "| excitation energy corrections included:" + head_corr = "| Excitation energy includes these corrections:" text += head_corr nspace = len(separator) - len(head_corr) - 1 text += nspace * " " + "|\n" diff --git a/adcc/backends/psi4.py b/adcc/backends/psi4.py index 70b3ec0d..94b3aa24 100644 --- a/adcc/backends/psi4.py +++ b/adcc/backends/psi4.py @@ -108,7 +108,7 @@ def pe_energy(self, dm, elec_only=True): @property def excitation_energy_corrections(self): - ret = {} + ret = [] if self.environment == "pe": ptlr = EnergyCorrection( "pe_ptlr_correction", @@ -120,9 +120,8 @@ def excitation_energy_corrections(self): lambda view: self.pe_energy(view.state_diffdm_ao, elec_only=True) ) - ret["pe_ptlr_correction"] = ptlr - ret["pe_ptss_correction"] = ptss - return ret + ret.extend([ptlr, ptss]) + return {ec.name: ec for ec in ret} @property def environment(self): diff --git a/adcc/backends/pyscf.py b/adcc/backends/pyscf.py index dcbfdc78..90128750 100644 --- a/adcc/backends/pyscf.py +++ b/adcc/backends/pyscf.py @@ -127,7 +127,7 @@ def pe_energy(self, dm, elec_only=True): @property def excitation_energy_corrections(self): - ret = {} + ret = [] if self.environment == "pe": ptlr = EnergyCorrection( "pe_ptlr_correction", @@ -139,12 +139,8 @@ def excitation_energy_corrections(self): lambda view: self.pe_energy(view.state_diffdm_ao, elec_only=True) ) - # NOTE: I don't like the duplicate 'name', - # but this way it's easier to exctract the corrections - # directly - ret["pe_ptlr_correction"] = ptlr - ret["pe_ptss_correction"] = ptss - return ret + ret.extend([ptlr, ptss]) + return {ec.name: ec for ec in ret} @property def environment(self): diff --git a/adcc/backends/veloxchem.py b/adcc/backends/veloxchem.py index ef08fec6..0f432ff6 100644 --- a/adcc/backends/veloxchem.py +++ b/adcc/backends/veloxchem.py @@ -131,7 +131,7 @@ def pe_energy(self, dm, elec_only=True): @property def excitation_energy_corrections(self): - ret = {} + ret = [] if hasattr(self.scfdrv, "pe_drv"): ptlr = EnergyCorrection( "pe_ptlr_correction", @@ -143,9 +143,8 @@ def excitation_energy_corrections(self): lambda view: self.pe_energy(view.state_diffdm_ao, elec_only=True) ) - ret["pe_ptlr_correction"] = ptlr - ret["pe_ptss_correction"] = ptss - return ret + ret.extend([ptlr, ptss]) + return {ec.name: ec for ec in ret} def get_backend(self): return "veloxchem" diff --git a/adcc/test_AdcMatrix.py b/adcc/test_AdcMatrix.py index 62b02441..ea5d0b92 100644 --- a/adcc/test_AdcMatrix.py +++ b/adcc/test_AdcMatrix.py @@ -229,20 +229,19 @@ def test_extra_term(self): with pytest.raises(ValueError): adcc.AdcMatrix("adc2", ground_state, diagonal_precomputed=matrix.diagonal() + 42) + with pytest.raises(TypeError): + AdcExtraTerm(matrix, "fail") + with pytest.raises(TypeError): + AdcExtraTerm(matrix, {"fail": "not_callable"}) shift = -0.3 shifted = AdcMatrixShifted(matrix, shift) - # TODO: need to do this to differentiate between + # TODO: need to use AmplitudeVector to differentiate between # diagonals for ph and pphh # if we just pass numbers, i.e., shift - # we get 2*shift on the diagonal :( + # we get 2*shift on the diagonal ones = matrix.diagonal().ones_like() - with pytest.raises(TypeError): - AdcExtraTerm(matrix, "fail") - with pytest.raises(TypeError): - AdcExtraTerm(matrix, {"fail": "not_callable"}) - def __shift_ph(hf, mp, intermediates): def apply(invec): return adcc.AmplitudeVector(ph=shift * invec.ph) @@ -260,6 +259,7 @@ def apply(invec): # cannot add to 'pphh_pphh' in ADC(1) matrix with pytest.raises(ValueError): matrix_adc1 += extra + shifted_2 = matrix + extra shifted_3 = extra + matrix for manual in [shifted_2, shifted_3]: diff --git a/adcc/workflow.py b/adcc/workflow.py index 8165ceab..a290032e 100644 --- a/adcc/workflow.py +++ b/adcc/workflow.py @@ -522,7 +522,7 @@ def setup_environment(matrix, environment): raise InputError( "Environment found in reference state, but no environment" " configuration specified. Please select from the following" - f" schemes: {valid_envs}." + f" schemes: {valid_envs} or set to False." ) elif environment and not hf.environment: raise InputError( @@ -530,17 +530,19 @@ def setup_environment(matrix, environment): " was found in reference state." ) elif not hf.environment: - environment = False - - if isinstance(environment, bool): - environment = {"ptss": True, "ptlr": True} if environment else {} - elif isinstance(environment, list): - environment = {k: True for k in environment} - elif isinstance(environment, str): - environment = {environment: True} - elif not isinstance(environment, dict): - raise TypeError("Invalid type for environment parameter" - f"' {type(environment)}'.") + environment = {} + + convertor = { + bool: lambda value: {"ptss": True, "ptlr": True} if value else {}, + list: lambda value: {k: True for k in value}, + str: lambda value: {value: True}, + dict: lambda value: value, + } + conversion = convertor.get(type(environment), None) + if conversion is None: + raise TypeError("Cannot convert environment parameter of type" + f"'{type(environment)}' to dict.") + environment = conversion(environment) if any(env not in valid_envs for env in environment): raise InputError("Invalid key specified for environment." diff --git a/docs/calculations.rst b/docs/calculations.rst index 71e60995..478e3a56 100644 --- a/docs/calculations.rst +++ b/docs/calculations.rst @@ -776,6 +776,37 @@ b) with the linear response scheme. The results of both schemes are then printed print(state_lr.describe()) +The output of the last two lines is:: + + +--------------------------------------------------------------+ + | adc2 singlet , converged | + +--------------------------------------------------------------+ + | # excitation energy osc str |v1|^2 |v2|^2 | + | (au) (eV) | + | 0 0.1434972 3.904756 0.0000 0.9187 0.08128 | + | 1 0.1554448 4.229869 0.0000 0.9179 0.08211 | + | 2 0.2102638 5.721569 0.0209 0.8977 0.1023 | + | 3 0.2375643 6.464453 0.6198 0.9033 0.09666 | + | 4 0.2699134 7.344718 0.0762 0.8975 0.1025 | + +--------------------------------------------------------------+ + | Excitation energy includes these corrections: | + | - pe_ptss_correction | + | - pe_ptlr_correction | + +--------------------------------------------------------------+ + + +--------------------------------------------------------------+ + | adc2 singlet , converged | + +--------------------------------------------------------------+ + | # excitation energy osc str |v1|^2 |v2|^2 | + | (au) (eV) | + | 0 0.1435641 3.906577 0.0000 0.9187 0.08128 | + | 1 0.1555516 4.232775 0.0000 0.9179 0.08211 | + | 2 0.210272 5.721794 0.0212 0.8977 0.1023 | + | 3 0.2378427 6.47203 0.6266 0.9034 0.09663 | + | 4 0.2698889 7.34405 0.0805 0.898 0.102 | + +--------------------------------------------------------------+ + + Further examples and details ---------------------------- Some further examples can be found in the ``examples`` folder diff --git a/libadcc/TensorImpl.hh b/libadcc/TensorImpl.hh index a1caf30d..6d3391b6 100644 --- a/libadcc/TensorImpl.hh +++ b/libadcc/TensorImpl.hh @@ -110,7 +110,7 @@ class TensorImpl : public Tensor { std::string describe_expression(std::string stage = "unoptimised") const override; /** Convert object to btensor for use in libtensor functions. */ - explicit operator libtensor::btensor &() { return *libtensor_ptr(); } + explicit operator libtensor::btensor&() { return *libtensor_ptr(); } /** Return inner btensor object *