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

[WIP] Control where additional nprop steps are added in the lambda protocol #156

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sgill2
Copy link
Collaborator

@sgill2 sgill2 commented Sep 28, 2018

Description

Adds a propRegion option to the integrator to allow focusing the additional propagation steps either in the sterics (in the middle) or the electrostatics (at either ends of the lambda protocol)

Todos

Notable points that this PR has either accomplished or will accomplish.

  • Adds a propRegion argument to the integrator
  • Updates TestNCMCSteps to account for the change in region
  • Adds tests to ensure that the steps are being added to the appropriate region

Status

  • Ready to go

@codecov
Copy link

codecov bot commented Sep 28, 2018

Codecov Report

Merging #156 into master will increase coverage by 1.36%.
The diff coverage is 79.16%.

@@            Coverage Diff            @@
##           master    #156      +/-   ##
=========================================
+ Coverage   59.53%   60.9%   +1.36%     
=========================================
  Files           7       7              
  Lines        1478    1504      +26     
  Branches      266     272       +6     
=========================================
+ Hits          880     916      +36     
+ Misses        498     491       -7     
+ Partials      100      97       -3
Impacted Files Coverage Δ
blues/reporters.py 41% <100%> (+1.76%) ⬆️
blues/utils.py 54.23% <70.58%> (+5.16%) ⬆️
blues/integrators.py 89.52% <82.14%> (-1.49%) ⬇️
blues/simulation.py 76.9% <0%> (+1.57%) ⬆️

@davidlmobley
Copy link
Member

@sgill2 - what needs to be done to finalize this? I believe it's working for @bergazin ?

Also this would INCREASE code coverage which is good. :)

@bergazin
Copy link
Member

bergazin commented Oct 7, 2018

Yes, and then perhaps including some textfile that lists what changes to the alchemical function someone could use to control how much time is spent on the sterics/electrostatics ?

@davidlmobley
Copy link
Member

In other words the documentation needs updating... :)

@bergazin
Copy link
Member

bergazin commented Oct 7, 2018

ie
40% electrostatics, 60% sterics (Default)
Lambda sterics: min(1, (1/0.3)abs(λ-0.5))
Lambda electrostatics: step(0.2-λ) - 1/0.2
λstep(0.2-λ) + 1/0.2(λ-0.8)*step(λ-0.8)

65% electrostatics, 35% sterics
Lambda sterics: min(1, (1/0.175)abs(λ-0.5)
Lambda electrostatics: step(0.325-λ) - 1/0.325
λ step(0.325-λ) + 1/0.325(λ-0.675)*step(λ-0.675)

90% electrostatics, 10% sterics
Lambda sterics: min(1, (1/0.05)abs(λ-0.5)
Lambda electrostatics: step(0.45-λ) - 1/0.45
λstep(0.45-λ) + 1/0.45(λ-0.55)*step(λ-0.55)

will be usefull if someone needs to use it for water....

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