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.

Sign up for GitHub

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

Jump to bottom

Add shift_coord for gro writer #730

Merged
merged 19 commits into from
Sep 7, 2023
Merged

Add shift_coord for gro writer #730

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4113f24
add shift_coord option
daico007 Jun 5, 2023
512c228
Merge branch 'main' into gro_writer_bug
daico007 Jun 13, 2023
930aed7
Merge branch 'main' into gro_writer_bug
daico007 Jun 27, 2023
9aa91f5
Merge branch 'main' into gro_writer_bug
daico007 Jul 7, 2023
983fd8f
Merge branch 'main' into gro_writer_bug
daico007 Jul 7, 2023
b12af25
add simple test to improve coverage
daico007 Jul 7, 2023
ca46c49
Merge branch 'main' of https://github.com/mosdef-hub/gmso into gro_wr…
daico007 Jul 9, 2023
58562bd
Merge branch 'main' into gro_writer_bug
daico007 Jul 10, 2023
3836ea1
Merge branch 'main' into gro_writer_bug
daico007 Jul 31, 2023
80cba27
Merge branch 'main' into gro_writer_bug
daico007 Aug 13, 2023
8454dab
Update gmso/formats/gro.py
daico007 Aug 28, 2023
2ef6eec
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 28, 2023
344d573
add additional check for test_write_gro_with_shift_coord
daico007 Aug 28, 2023
24be7e1
change precision to n_decimals
daico007 Aug 28, 2023
13361b0
Merge branch 'main' of https://github.com/mosdef-hub/gmso into gro_wr…
daico007 Aug 28, 2023
2898d88
Merge branch 'main' into gro_writer_bug
daico007 Sep 6, 2023
cd7de1f
vectorize shift_coord
daico007 Sep 6, 2023
a95eae7
Merge branch 'gro_writer_bug' of https://github.com/daico007/gmso int…
daico007 Sep 6, 2023
5525350
address remaining comment
daico007 Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions gmso/formats/gro.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def read_gro(filename):


@saves_as(".gro")
def write_gro(top, filename, precision=3):
def write_gro(top, filename, precision=3, shift_coord=False):
"""Write a topology to a gro file.

The Gromos87 (gro) format is a common plain text structure file used
Expand All @@ -121,6 +121,9 @@ def write_gro(top, filename, precision=3):
The location and name of file to save to disk.
precision : int, optional, default=3
daico007 marked this conversation as resolved.
Show resolved Hide resolved
The number of sig fig to write out the position in.
shift_coord : bool, optional, default=False
If True, shift the coordinate to be all sites to ensure
all coordinates are non-negative.
daico007 marked this conversation as resolved.
Show resolved Hide resolved

Notes
-----
Expand All @@ -131,7 +134,8 @@ def write_gro(top, filename, precision=3):

"""
pos_array = np.ndarray.copy(top.positions)
pos_array = _validate_positions(pos_array)
if shift_coord:
pos_array = _validate_positions(pos_array)
Copy link
Contributor

Choose a reason for hiding this comment

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

I was glancing at this function and feel like it should be rewritten to use vectorized operations instead of this a for loop.

def _validate_positions(pos_array):
    """Modify coordinates, as necessary, to fit limitations of the GRO format."""
    if np.min(pos_array) < 0:
        warnings.warn(
            "Topology contains some negative positions. Translating "
            "in order to ensure all coordinates are non-negative."
        )
    min_xyz = np.min(x, axis=1) # smallest position of each coordinate
    min_xyz0 =  np.where(min_xyz<0, min_xyz, 0) #shift by 0 or minimum value of the positions in that coordinate
    pos_array -= min_xyz0
    return pos_array

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as below comment. I think it's worthwhile to make these changes to this function as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh shoot, I somehow miss these, I will add these changes in


with open(filename, "w") as out_file:
out_file.write(
Expand Down
4 changes: 4 additions & 0 deletions gmso/tests/test_gro.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ def test_write_gro(self):
top = from_parmed(pmd.load_file(get_fn("ethane.gro"), structure=True))
top.save("out.gro")

def test_write_gro_with_shift_coord(self):
top = from_parmed(pmd.load_file(get_fn("ethane.gro"), structure=True))
top.save("out.gro", shift_coord=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

read_top = Topology("out.gro")
assert np.all(list(map(lambda x: x.positions>=0, read_top.sites)))

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how these comments got resolved, @daico007 did you address them somehow?


def test_write_gro_non_orthogonal(self):
top = from_parmed(pmd.load_file(get_fn("ethane.gro"), structure=True))
top.box.angles = u.degree * [90, 90, 120]
Expand Down
Loading