Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
GAMESS EFP Support #256
base: master
Are you sure you want to change the base?
GAMESS EFP Support #256
Changes from all commits
16b90b8
6114dba
b957f46
1291e9e
8adce37
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice for now, may have to revisit in future. i'm against a 3rd labels set. https://github.com/loriab/pylibefp/blob/master/pylibefp/wrapper.py#L961-L966
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted, that's going to be a headache later on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless something needs it, maybe drop the current ref line. it's questionable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to add that line since
CURRENT REFERENCE ENERGY
is expected here:QCEngine/qcengine/programs/gamess/runner.py
Line 155 in 4525eb3
What exactly is questionable about it? Would setting
CURRENT ENERGY
make more sense? DoesCURRENT REFERENCE ENERGY
imply a full QM calculation?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you could get away with
CURRENT ENERGY
only that'd be better, but ifCURRENT REFERENCE ENERGY
needed, fine. It's questionable b/c it's an interaction energy like SAPT rather than a total energy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's in
molecule.extras["efp"]
again? I don't recall it from MolSSI/QCElemental#124 (comment) (though that needn't be a strict blueprint).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose I should rename that variable from
molecule.extras["efp"]
tomolecule.extras["efp_molecule"]
in order to be more consistent with the blueprint. In the blueprint,molecule.extras["efp_molecule"]
is a schema-like dictionary defining all of the fragment locations and potentials. In this PR,molecule.extras["efp_molecule"]
is instead expected to be an equivalent GAMESS-ready string, which is directly inserted into the input file.This is a short-tem solution. Eventually, we need an input-parsing function that takes in your
efp_molecule
and converts it to the GAMESS-ready string. I held off on doing this right away because I wasn't sure how final the blueprint was.I added an example script to the PR description that demonstrates how you'd run MAKEFP and then get an EFP interaction energy. This will give you a better idea of what the
efp_molecule
string looks like in this PR.