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

Linear response with solvent #471

Merged
merged 20 commits into from
Nov 24, 2023
Merged

Conversation

robertodr
Copy link
Contributor

Rehash of #456

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (887a887) 69.25% compared to head (2a828ae) 69.90%.

Files Patch % Lines
src/environment/SCRF.cpp 80.95% 4 Missing ⚠️
src/environment/StepFunction.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #471      +/-   ##
==========================================
+ Coverage   69.25%   69.90%   +0.65%     
==========================================
  Files         181      186       +5     
  Lines       14977    15025      +48     
==========================================
+ Hits        10372    10503     +131     
+ Misses       4605     4522      -83     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@robertodr
Copy link
Contributor Author

After fixing a bug with handling of default radii (I opened a separate PR with just that fix) everything, nonequilibrium solvation included, seems to run smoothly and match up with GTO results from G16. I need to add a couple of tests and then it's good to go.

python/template.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@Gabrielgerez Gabrielgerez left a comment

Choose a reason for hiding this comment

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

I left some questions and comments. I really like how the ReactionPotential looks now. I mentioned the naming of the outside section, but it is not that essential besides aesthetics.

src/driver.cpp Outdated Show resolved Hide resolved
src/environment/SCRF.h Show resolved Hide resolved
Copy link
Contributor

@Gabrielgerez Gabrielgerez left a comment

Choose a reason for hiding this comment

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

I like the new naming of the outside section of the input. besides the comment on the .gitignore everything looks good to me

.gitignore Outdated Show resolved Hide resolved
@robertodr robertodr merged commit 9de2280 into MRChemSoft:master Nov 24, 2023
@robertodr robertodr deleted the pcm-terms-response branch November 24, 2023 12:51
gitpeterwind pushed a commit to gitpeterwind/mrchem that referenced this pull request Aug 5, 2024
* Use C++17

* Use std::tie to unpack tuple return value

* Remove some cruft from Cavity and Permittivity

* SCRF deals in densities only, not orbitals

* Refactor Reaction{Potential,Operator} following Coulomb example

In preparation for the computation of the response terms, we make ReactionPotential abstract and add
subclass ReactionPotentialD1 to deal with one single set of orbitals.

* Modify input to accept static and dynamic permittivities

* Handle nonequilibrium solvation more gracefully

* Sign change was necessary

* Update input parser

* Small fixes to include order

* Rename section for permittivity outside

* Add tests
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