search

adc-connect / adcc

adcc: Seamlessly connect your program to ADC
https://adc-connect.org
GNU General Public License v3.0
32 stars 19 forks source link

Updates for solvent models (new user interface, new schemes) #123

Closed maxscheurer closed 3 years ago

maxscheurer commented 3 years ago

Preliminary PR, just to stash some work...

maxscheurer commented 3 years ago

The only reason this PR is not moving forward is that I've not been able to come up with a user-friendly solution for integrating this feature properly.

The following feature scenarios are possible:

  1. "pt scheme": purely perturbative energy corrections (currently automatically computed and incorporated into excitation_energy if a PE ground state is present. Uncorrected excitation energies (i.e., the "actual" eigenvalues") are in excitation_energy_uncorrected
  2. "postSCF scheme": coupling to the environment via an additional (excitonic) coupling term (via CIS-like transition density). In this case, the excitation_energy_uncorrected values already contain the "linear response" contribution, so the ptLR does not make sense anymore. The ptSS still makes sense. The eigenvalues of this modified ADC matrix however still correspond to excitation_energy_uncorrected. At the moment, both ptLR and ptSS corrections are always computed, also for the "postSCF scheme", which makes little sense.
  3. "ground state": don't couple at all in ADC, just through the PE-HF ground state. This is currently not possible if a PE-HF ground state is given.

The same problematic will hold for other solvent models (PCM for example). So the following questions arise:

  1. Which of the above methods should be the default? (I'd say 1. because for excitation energies it offers the best trade-off between performance and accuracy)
  2. How does the user select the coupling scheme? This must be possible both from run_adc and via "matrix construction".
  3. How do we implement the "logic" of disabling corrections etc. if a certain scheme is chosen. This is the most annoying part, because we'd like to provide transparent result to the user.

Just wanted to leave this here for discussion, because I have not made any progress alone so far...

lgtm-com[bot] commented 3 years ago

This pull request introduces 1 alert when merging de5bbb93d9a286aca75d8a2a40e409859c43d26e into 041cdcb16ec60d1655db8d562ef2f3b53038c120 - view on LGTM.com

new alerts:

maxscheurer commented 3 years ago

I hate clang-format, why is it changing this one file anyways? I've added the .clang-format file we used to have in adccore, but it does not seem to care. I cannot reproduce this locally, however 😠

mfherbst commented 3 years ago

Different clang-format versions? Sometimes this happens then.

maxscheurer commented 3 years ago

Different clang-format versions? Sometimes this happens then.

I've tried both v11 and v9 on CI, both giving the same diff... I can make the change such that the CI runs without failure.

mfherbst commented 3 years ago

I would not get too much held up with this. either make the change or disable the relevant line from formatting or sth like that.

maxscheurer commented 3 years ago

I think we have quite the functionality we want now! If you got a couple of minutes in the next days, @mfherbst, it would be nice if you could give it a brief look. I'm now working on refactoring the user-facing changes in run_adc and some convenience wrappers, and then we can do refactoring etc. 🚀

maxscheurer commented 3 years ago

Everything seems to be operational again 🎉

maxscheurer commented 3 years ago

I've refactored the stuff we discussed 😄 The interface currently has a bit more flexibility, but it feels good from a user perspective imho. Let me know what you think @mfherbst 🧐

maxscheurer commented 3 years ago

Docs now have a section on performing PE calculations 🚀 It's all coming together... 😄

mfherbst commented 3 years ago

@maxscheurer Noted it. Not sure when I get to it. Have a deadline next week.

maxscheurer commented 3 years ago

Sure, I didn't want to be too annoying, sorry 😄 Then I'll start working on sth else and we move this forward when you have more time.

mfherbst commented 3 years ago

Don't worry ... I just wanted you to know.

mfherbst commented 3 years ago

@maxscheurer Unless you want to rebase I would squash-merge once tests pass.

maxscheurer commented 3 years ago

Squash-merge is fine 😄 🚀

maxscheurer commented 3 years ago

I guess a release of v0.15.9 would be adequate?