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

adds gcp harness #278

Merged
merged 1 commit into from
Feb 11, 2021
Merged

adds gcp harness #278

merged 1 commit into from
Feb 11, 2021

Conversation

hokru
Copy link
Contributor

@hokru hokru commented Nov 2, 2020

Description

Adds harness for the gcp program that calculates a geometrical counter poise correction.
The method's parametrization depends on the basis set and (to a less extent) on the Hamiltonian (HF or DFT).
The manual or help from the binary lists what combinations are available. The gCP part for the 3c correction is also accessible (method = hf3c or pbeh3c). Minimal example:

task = qcel.models.AtomicInput (
    molecule= mol,
    driver="gradient",
    model={"method": "HF/SV"},
)
 qcengine.compute(task,"gcp")

There are already some provisions in place for a planned update of the gcp code for a smoother interoperability (version output, better custom param file support). Note that the current, uncommon version 2.02 gets changed to 2.2 somewhere.

Changelog description

Addition of gcp harness.

Status

  • Code base linted
  • tests added
  • Ready to go

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Nov 2, 2020

This pull request introduces 3 alerts when merging 78618a6 into a206fb9 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Variable defined multiple times

@codecov
Copy link

codecov bot commented Nov 3, 2020

Codecov Report

Merging #278 (e250752) into master (3cbe78b) will increase coverage by 0.55%.
The diff coverage is 86.88%.

qcengine/programs/gcp.py Show resolved Hide resolved
qcengine/programs/tests/test_dftd3_mp2d.py Show resolved Hide resolved
docs/source/program_overview.rst Outdated Show resolved Hide resolved
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Nov 5, 2020

This pull request introduces 1 alert when merging af554ff into a206fb9 - view on LGTM.com

new alerts:

  • 1 for Unused import

Copy link
Collaborator

@loriab loriab left a comment

Choose a reason for hiding this comment

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

no need to add to psi.yaml, imo

qcengine/programs/gcp.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@loriab loriab left a comment

Choose a reason for hiding this comment

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

lgtm, thanks! let me know when you're content to merge.

@loriab
Copy link
Collaborator

loriab commented Feb 8, 2021

I neglected to pursue getting this merged before the SAPT-D PR generated conflicts. Would you rebase, please, and then let me know if ready to merge? Thanks!

@hokru
Copy link
Contributor Author

hokru commented Feb 10, 2021

Yeah I also lost track of this, sorry. Rebase was a bit messy.

Not 100% sure about official gcp updates right now that the harness should support already.

@loriab
Copy link
Collaborator

loriab commented Feb 10, 2021

thanks! I'll get this merged, then straighten out CI on GHA. do you want to be added to codemeta? I can copy your entry over from psi.

@hokru
Copy link
Contributor Author

hokru commented Feb 10, 2021

Yes, please add and copy from psi, thanks.

@awvwgk
Copy link
Contributor

awvwgk commented Feb 10, 2021

Note that the current, uncommon version 2.02 gets changed to 2.2 somewhere.

I might have screwed up the versioning in the new gcp release when putting up a version 2.1.0...

@loriab
Copy link
Collaborator

loriab commented Feb 11, 2021

Note that the current, uncommon version 2.02 gets changed to 2.2 somewhere.

I might have screwed up the versioning in the new gcp release when putting up a version 2.1.0...

I think it's ok because the binary names changed, and detection relies upon which gcp https://github.com/MolSSI/QCEngine/blob/master/qcengine/testing.py#L148

@awvwgk
Copy link
Contributor

awvwgk commented Feb 11, 2021

Okay, I'll probably bump the version to 2.3 for the next release to be on the safe side.

@loriab loriab merged commit 5fb99be into MolSSI:master Feb 11, 2021
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.

3 participants