Skip to content

Convert AMBER input files to DLPOLY input files#1102

Closed
vwcruzeiro wants to merge 17 commits into
ParmEd:masterfrom
vwcruzeiro:master
Closed

Convert AMBER input files to DLPOLY input files#1102
vwcruzeiro wants to merge 17 commits into
ParmEd:masterfrom
vwcruzeiro:master

Conversation

@vwcruzeiro

Copy link
Copy Markdown
Contributor

This PR adds to ParmEd the capability to convert AMBER input files to DLPOLY input files. The opposite process is impossible because the topology file in DLPOLY does not contain information about residues, only about "molecules" (in DLPOLY, one "molecule" cannot have bonds with any other "molecule").

This PR also adds tests for the new functionality.

Vinicius Wilian D. Cruzeiro added 11 commits November 18, 2018 11:02
This reverts commit 9dd2577.
into DLPOLY input files. In this commit I set the structural basis
for this changes (placed new files at parmed/dlpoly which were
adapted from parmed/gromacs), and I have an initial version of
the dlpolyconfig.py file that is already giving the expected
results. I still need to address the generation of the FIELD file
and other details
can get the velocities from the Amber inpcrd file, in case it is there.
Comment thread parmed/amber/netcdffiles.py Outdated
if 'velocities' in self._ncfile.variables:
vels = self._ncfile.variables['velocities'][:]
return (vels.reshape((-1, self.atom, 3)) * self.velocity_scale)
return (vels.reshape((1, self.atom, 3)) * self.velocity_scale)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should be -1 because there may be more than 1 frame. The -1 indicates that the dimension should take whatever value necessary to give a shape that accounts for the full length of the array. For NetCDF trajectories that have more than 1 frame, this will fail.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@swails , makes sense. Thank you for carefully checking on this.

pyflakes is detecting failures and for this reason it is killing the Travis CI build. What should I do to fix this? Just remove the "imported but unused" stuff?

Thanks!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, resolve the pyflakes issues.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@swails : continuing on the first comment you made, I noticed that a few lines above ( parmed/amber/netcdffiles.py#255 ) there is an equivalent function for the coordinates, and there is no -1 inside the reshape function:

            return coords.reshape((1, self.atom, 3))

This is why I made the change you commented in the velocities function. Shall I place a -1 in this line as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I think that's a good idea.

Comment thread parmed/dlpoly/__init__.py Outdated
Comment on lines +4 to +5
import os as _os
from parmed.utils import which as _which

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hi, what's the aim of this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I implemented the functionalities on this PR starting from what was already in ParmEd for Gromacs. I am not sure why these as choices were employed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It prevents them from being imported with from parmed.dlpoly import *

@vwcruzeiro

Copy link
Copy Markdown
Contributor Author

@swails : apparently the tests that failed were complaining about the following: Could not find netCDF4 module. How can we fix that?

@vwcruzeiro

Copy link
Copy Markdown
Contributor Author

Hello @swails , just making sure my last comment did not get lost. I am unsure how to proceed regarding the errors in Travis CI. These appear to be unrelated to my changes in this PR. Thanks.

@swails

swails commented Aug 20, 2020

Copy link
Copy Markdown
Contributor

Hello @swails , just making sure my last comment did not get lost. I am unsure how to proceed regarding the errors in Travis CI. These appear to be unrelated to my changes in this PR. Thanks.

Yea, I saw it. The problem is that the netcdf bundled with ParmEd (taken from scipy) uses deprecated features. I updated to the latest version of that library in #1104. As long as that passes the tests (i.e., it doesn't break Python 2.7 or something else), then we can merge that and update your branch with the fix.

@swails

swails commented Oct 8, 2020

Copy link
Copy Markdown
Contributor

Finally fixed the NetCDF/Travis issues.

@swails

swails commented Oct 8, 2020

Copy link
Copy Markdown
Contributor

The opposite process is impossible because the topology file in DLPOLY does not contain information about residues, only about "molecules" (in DLPOLY, one "molecule" cannot have bonds with any other "molecule").

There's no reason we can't map a molecule from DLPOLY to a residue in Amber. The main target functionality here is to get the same energies and forces in both programs. If the informatics portion doesn't map between the two programs I don't really care. For actual informatics applications, neither DLPOLY or AMBER input files will be used.

@swails swails left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Relative imports are preferable because they permit easier relocation/subpackaging of ParmEd.

Comment thread parmed/dlpoly/__init__.py Outdated
Comment thread parmed/dlpoly/__init__.py Outdated
Comment thread parmed/dlpoly/dlpolyconfig.py Outdated
Comment thread parmed/dlpoly/dlpolyfield.py Outdated
@vwcruzeiro

Copy link
Copy Markdown
Contributor Author

Thank you very much for your comments @swails . I have made the changes you suggested, and I also pushed your recent changes. Let's hope all tests will pass now.

@vwcruzeiro

Copy link
Copy Markdown
Contributor Author

All test are passing now 👍

@vwcruzeiro

Copy link
Copy Markdown
Contributor Author

Is this ready to go? :D

@vwcruzeiro

Copy link
Copy Markdown
Contributor Author

Hello @swails . Just a friendly reminder about this pull request. Happy Thanksgiving!

@swails swails mentioned this pull request Feb 16, 2021
@swails

swails commented Feb 16, 2021

Copy link
Copy Markdown
Contributor

I'm closing this in favor of #1146. I created a branch from this PR, merged master, and refactored to get rid of the code duplication from GROMACS.

@vwcruzeiro, this refactor continues to pass the tests you added. Can you give it a quick look and I'll merge when you give the go-ahead?

@swails swails closed this Feb 16, 2021
@vwcruzeiro

Copy link
Copy Markdown
Contributor Author

@swails : I confirm that it is working. Thank you!

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