IP/EA ADC(0-3) implementation without properties - #206
Conversation
… eigenvectors only for IP/EA ADC
|
Please rebase onto the latest |
frieschneider
left a comment
frieschneider
left a comment
There was a problem hiding this comment.
I looked through the Python files, but not yet the C++ ones.
jonasleitner
left a comment
jonasleitner
left a comment
There was a problem hiding this comment.
I went through the python files only. I try to continue with the cpp part once I find the time :)
| # | ||
| __all__ = ["block"] | ||
|
|
||
| AdcBlock = namedtuple("AdcBlock", ["apply", "diagonal"]) |
There was a problem hiding this comment.
I don't like defining AdcBlock three times. Maybe we can add the definition to block.py or AdcMatrix.py and import it for PP, IP, and EA. Also we probably want it rather to be a dataclass.
| # | ||
| def block_p_p_2(hf, mp, intermediates): | ||
| # Intermediate can be found in 'adc_pp/matrix.py' | ||
| i1 = intermediates.adc2_i1 |
There was a problem hiding this comment.
I think adc2_i1 (and adc2_i2) should be renamed to ea/ip_adc2_m11 and moved to the ip/ea matrix.py
| # 3rd order main | ||
| # | ||
| def block_p_p_3(hf, mp, intermediates): | ||
| i1 = intermediates.adc3_ea_i1 |
There was a problem hiding this comment.
Please rename the intermediate to ea_adc3_m11
| @register_as_intermediate | ||
| def sigma_inf_vv(hf, mp, intermediates): | ||
| # Static self-energy, oo part \Sigma_{ij}(\infty) | ||
| p0 = mp.mp2_diffdm | ||
| return (einsum("iajb,ij->ab", hf.ovov, p0.oo) | ||
| + 2 * einsum("iacb,ic->ab", hf.ovvv, p0.ov) | ||
| + einsum("acbd,cd->ab", hf.vvvv, p0.vv)).symmetrise() |
There was a problem hiding this comment.
I think this definition should be implemented on the GroundState class similar to sigma_inf_ov.
| raise ValueError("ADC(0) level and " | ||
| "beyond expects an excitation amplitude with a " | ||
| "singles part.") |
There was a problem hiding this comment.
Feels weird to duplicate like all the code from adc_pp.util.py twice. Maybe we can avoid that and use one common implementation that lives somewhere else - not in the adc_pp, adc_ip, adc_ea modules?
| else: | ||
| n_guesses_per_state = 1 |
There was a problem hiding this comment.
This should be an explicit check for lanczos with an else that raises an exception
| n_states: int, | ||
| kind: str, | ||
| spin_change: float, | ||
| n_guesses: int, |
There was a problem hiding this comment.
| n_guesses: int, | |
| n_guesses: Optional[int], |
| matrix : AdcMatrix | ||
| n_states : int | ||
| guesses : list[AmplitudeVector] | ||
| A list of guess vectors | ||
| kind : str |
There was a problem hiding this comment.
Please also add the remaining arguments :)
| kstr = "" | ||
| if kind != "any": | ||
| kstr = " " + kind |
There was a problem hiding this comment.
I am not sure about removing the space here 😅
It would be way easier and cleaner to collect all required information in a list anyway and simply use " ".join for the formatting.
| #!/usr/bin/env python3 | ||
| import pytest | ||
|
|
||
| from adcc.AdcMethod import AdcMethod, AdcType | ||
| from adcc.ChargedExcitations import DetachedStates, AttachedStates | ||
|
|
||
| from .testdata_cache import testdata_cache | ||
|
|
||
|
|
||
| cases_ip_ea = [ | ||
| ("h2o_sto3g", "ip-adc2", "gen", "doublet"), | ||
| ("h2o_sto3g", "ea-adc2", "gen", "doublet"), | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("system,method,case,kind", cases_ip_ea) | ||
| def test_ip_ea_basic_interface(system, method, case, kind): | ||
| adc_type = AdcMethod(method).adc_type | ||
|
|
||
| if adc_type is AdcType.IP: | ||
| state = testdata_cache.adcc_states( | ||
| system=system, | ||
| method=method, | ||
| case=case, | ||
| kind=kind, | ||
| is_alpha=True, | ||
| ) | ||
| assert isinstance(state, DetachedStates) | ||
| elif adc_type is AdcType.EA: | ||
| state = testdata_cache.adcc_states( | ||
| system=system, | ||
| method=method, | ||
| case=case, | ||
| kind=kind, | ||
| is_alpha=True, | ||
| ) | ||
| assert isinstance(state, AttachedStates) | ||
| else: | ||
| raise AssertionError("Unexpected ADC type") | ||
|
|
||
| # size matches number of excitation vectors | ||
| assert state.size == len(state.excitation_vector) | ||
| assert state.size == len(state.excitation_energy) |
There was a problem hiding this comment.
I don't get the purpose of this test. I think we can remove this file?
3 participants
New methods on user side:
ip-adc{0..3}()
ea-adc{0..3}()
New keywords on user side:
n_doublets, is_alpha
e.g. ip_adc3(scf_result, n_doublets=5, is_alpha=True)
run_adc(method="ip-adc2", ...)
Remarks:
No simultaneous calculation of alpha and beta states to simplify the code and error handling and to keep consistency with PP calculations. If alpha and beta would be computed in the same adcc instance, intermediates could be reused.
But they can also be reused by providing the AdcMatrix instead of the HF data
Quartets not yet implemented since they are pure doubles states which are only Hartree-Fock orbital differences.