-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add pV and U to reduced potential when they are given #62
Merged
Merged
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
dcad2b8
Add pV and U to reduced potential when they are given
harlor 037bee4
Update TestStatisticalInefficiency after u_nk parser update
harlor 65159f7
If Potential energy is given add it to reduced potential
harlor 27cb260
Add tests for the calculation of the reduced potential
harlor 0af3093
Use assert_almost_equal instead of assert
harlor 837c266
Merge branch 'master' into reduced_potential_pr
harlor fe014b3
Use decimal precision 6 for float comparison in tests
harlor 886b0ef
Use reasonable decimal precision for float comparison in tests
harlor f8ad375
Compare with ordered datasets
harlor 74333c5
Remove unnecessary type conversion to float
harlor bfbe0b0
Merge branch 'master' into reduced_potential_pr
dotsdl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
Checking the files you provided @harlor (https://userpage.fu-berlin.de/dominikwille/nvt300K.tgz), I see "Potential Energy (kJ/mol)" in the
ti_0.xvg
file. Should we also try and match that?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.
@mrshirts explained that in #59 (comment). So I think we need the total energy.
I uploaded a dataset with total energy here: http://dominikwille.userpage.fu-berlin.de/nvt300K_u.tgz
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.
Okay, but I think depending on MDP options given to gromacs it's possible to have a dataset without "Total Energy" but with "Potential Energy". I was thinking that we could handle this in another PR with kwargs, but I don't believe this should be neglected.
Could you add a check for the existence of a "Potential Energy" column as you did for the "Total Energy" column, and if present, use this instead of the "Total Energy" column? That would satisfy me for the purposes of this PR