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

Amber TI parser should provide dimensionless dH/dl #57

Closed
3 tasks
orbeckst opened this issue Jan 22, 2018 · 3 comments · Fixed by #74
Closed
3 tasks

Amber TI parser should provide dimensionless dH/dl #57

orbeckst opened this issue Jan 22, 2018 · 3 comments · Fixed by #74
Assignees
Milestone

Comments

@orbeckst
Copy link
Member

The current implementation of the Amber TI parser (amber.extract_dHdl) returns a dataframe with native Amber units (as used to be the case in alchemical-analysis) instead of the prescribed reduced dHdl standard form: the potential energy derivatives should be divided by 1/kT.

See for example

dHdl = beta * dHdl

Todo:

  • amber.extract_dHdl() needs additional temperature T argument
  • make data dimensionless (something like dHdl = beta * dHdL with beta = 1/(kB*T)
  • adjust tests

(See also #56 )

@orbeckst orbeckst added this to the release 0.2.0 milestone Jan 22, 2018
@orbeckst
Copy link
Member Author

@shuail sorry, this slipped through when I was reviewing earlier. We need to make sure that all parsers produce the same dataframes, though, so this is a necessity.

@shuail
Copy link
Collaborator

shuail commented Jan 22, 2018

Got it, will take a look soon

@orbeckst orbeckst assigned dotsdl and unassigned shuail Feb 16, 2019
@orbeckst
Copy link
Member Author

@dotsdl I know you're busy (GitHub displays status nowadays?!?) but because we talked about it, I am tentatively assigning you to this issue and #56 . Maybe @shuail can give input when needed.

orbeckst added a commit that referenced this issue Apr 14, 2019
- fix #57 (TI parser)
- fix #56 (FEP parser)
- adjusted tests by explicitly converting the originally supplied
  energies in kcal/mol to kT
- TODO: No FEP tests for the Amber data that actually process the files
@orbeckst orbeckst self-assigned this Apr 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants