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

problems with gromacs parser in alchemical-free-energy #181

Open
vvoelz opened this issue Apr 18, 2015 · 12 comments
Open

problems with gromacs parser in alchemical-free-energy #181

vvoelz opened this issue Apr 18, 2015 · 12 comments

Comments

@vvoelz
Copy link

vvoelz commented Apr 18, 2015

We have found a problem in pymbar/examples/alchemical-free-energy/parsers/parser_gromacs.py with parsing large dhdl.xvg files. In slice_data(), there's a line:

f.len_first, f.len_last = (len(line.split()) for line in unixlike.tailPy(f.filename, 2))

the tailPy routine here is supposed to do a "tail" on the last two lines, and compare the number of fields; if they disagree, then the last line of the xvg file is corrupt and should be ignored.

In our case, we have found that unixlike.tailPy() gives incorrect results, truncating the last line. We suspect this may be because our xvg files have many alchemical intermediates (40 or more), all written in high-precision scientific notation, and unixlike.tailPy() has a character limit (?!)

def tailPy(f, nlines, LENB=1024): <------ ?

I think this could easily be fixed either rewriting unixlike.tailPy or just using the normal python read() or readlines(). --VInce

@jchodera
Copy link
Member

@mrshirts: Can you guys take a look at this?

@kyleabeauchamp
Copy link
Collaborator

I think there is an outstanding pull request already waiting for
review. We should see if that fixes the problem.

On 4/18/15, John Chodera notifications@github.com wrote:

@mrshirts: Can you guys take a look at this?


Reply to this email directly or view it on GitHub:
#181 (comment)

@mrshirts
Copy link
Collaborator

I emailed pavel (who wrote the code) to take a look at it.

On Sat, Apr 18, 2015 at 12:57 PM, Kyle Beauchamp notifications@github.com
wrote:

I think there is an outstanding pull request already waiting for
review. We should see if that fixes the problem.

On 4/18/15, John Chodera notifications@github.com wrote:

@mrshirts: Can you guys take a look at this?


Reply to this email directly or view it on GitHub:
#181 (comment)


Reply to this email directly or view it on GitHub
#181 (comment).

@kyleabeauchamp
Copy link
Collaborator

We still need someone to merge the pull request. I am not sufficiently familiar with the Gromacs code to review the changes.

@wesbarnett
Copy link

Hey I think the pull requests you mention were partly authored by me. Those were just a small fix to the graph output in the alchemical analysis script, not the actual gromacs parser, so I think this is a different issue.

@jchodera
Copy link
Member

@mrshirts: Who has primary responsibility for the gromacs parser at this point?

@vvoelz
Copy link
Author

vvoelz commented Apr 22, 2015

@wesbarnett -- not to add more confusion, but are you referring to an output problem where the "vdWaals" free energy is always zero? This is something we consistently get.


Coulomb: 654.143 +- 5.676 643.565 +- 6.049 35.005 +- 0.171 9.977 +- 0.165 21.819 +- 0.128 18.144 +- 0.293
vdWaals: 0.000 +- 5.676 0.000 +- 6.049 0.000 +- 0.000 0.000 +- 0.000 0.000 +- 0.000 0.000 +- 0.000
TOTAL: 654.143 +- 8.027 643.565 +- 8.554 35.005 +- 0.171 9.977 +- 0.165 21.819 +- 0.128 18.144 +- 0.293

@wesbarnett
Copy link

@vvoelz Yes, that's one of the fixes in the pull request.

@mrshirts
Copy link
Collaborator

Who has primary responsibility for the gromacs parser at this point?

I don't think we've entirely decided. Pavel, would you be able do the maintenance for now?

@davidlmobley
Copy link

Pavel should be handling this. I'm talking with him today. Sorry for the delay here. We're new to GitHub and I was not monitoring issues on here until now. @wesbarnett - worlds collide. :)

@davidlmobley
Copy link

@FEPanalysis (Pavel) and I talked. He is working through issue #179 and #180 and may need test data from @wesbarnett. He also needs permissions to handle merging of pull requests (or we need to appoint someone else who will do it) and we're (@mrshirts @jchodera) sorting that out via e-mail now.

He also has a fix to @vvoelz 's issue #181 and will submit it as a pull request once it's sorted out who is handling merges.

@davidlmobley
Copy link

@vvoelz - we think this is resolved by MobleyLab/alchemical-analysis#11. Can you check that it fixes your issue?

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

No branches or pull requests

6 participants